ecbff1
From 6930375c3f0d9681f27b42f1d0406721c3f9a013 Mon Sep 17 00:00:00 2001
40a46b
From: Lennart Poettering <lennart@poettering.net>
40a46b
Date: Mon, 2 Nov 2015 23:14:30 +0100
40a46b
Subject: [PATCH] sd-journal: various clean-ups and modernizations
40a46b
40a46b
- Always print a debug log message about files and directories we cannot
40a46b
  open right when it happens instead of the caller, thus reducing the
40a46b
  number of places where we need to generate the debug message.
40a46b
40a46b
- Always push the errors we encounter immediately into the error set,
40a46b
  when we run into them, instead of in the caller. Thus, we never forget
40a46b
  to push them in.
40a46b
40a46b
- Use stack instead of heap memory where we can.
40a46b
40a46b
- Make remove_file() void, since it cannot fail anyway and always
40a46b
  returned 0.
40a46b
40a46b
- Make local machine check of journal directories explicit in a
40a46b
  function, to make things more readable.
40a46b
40a46b
- Port to all directory listing loops FOREACH_DIRENT_ALL()
40a46b
40a46b
- sd-daemon is library code, hence never log at higher log levels than
40a46b
  LOG_DEBUG.
40a46b
40a46b
(cherry picked from commit d617408ecbe69db69aefddfcb10a6c054ea46ba0)
40a46b
40a46b
Related: #1465759
40a46b
---
40a46b
 src/journal/sd-journal.c | 242 ++++++++++++++++++++++-------------------------
40a46b
 1 file changed, 111 insertions(+), 131 deletions(-)
40a46b
40a46b
diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
40a46b
index 3749f9e89..9895d9608 100644
40a46b
--- a/src/journal/sd-journal.c
40a46b
+++ b/src/journal/sd-journal.c
40a46b
@@ -1171,6 +1171,8 @@ static bool file_has_type_prefix(const char *prefix, const char *filename) {
40a46b
 }
40a46b
 
40a46b
 static bool file_type_wanted(int flags, const char *filename) {
40a46b
+        assert(filename);
40a46b
+
40a46b
         if (!endswith(filename, ".journal") && !endswith(filename, ".journal~"))
40a46b
                 return false;
40a46b
 
40a46b
@@ -1195,7 +1197,7 @@ static bool file_type_wanted(int flags, const char *filename) {
40a46b
 
40a46b
 static int add_any_file(sd_journal *j, const char *path) {
40a46b
         JournalFile *f = NULL;
40a46b
-        int r;
40a46b
+        int r, k;
40a46b
 
40a46b
         assert(j);
40a46b
         assert(path);
40a46b
@@ -1204,20 +1206,23 @@ static int add_any_file(sd_journal *j, const char *path) {
40a46b
                 return 0;
40a46b
 
40a46b
         if (ordered_hashmap_size(j->files) >= JOURNAL_FILES_MAX) {
40a46b
-                log_warning("Too many open journal files, not adding %s.", path);
40a46b
-                return set_put_error(j, -ETOOMANYREFS);
40a46b
+                log_debug("Too many open journal files, not adding %s.", path);
40a46b
+                r = -ETOOMANYREFS;
40a46b
+                goto fail;
40a46b
         }
40a46b
 
40a46b
         r = journal_file_open(path, O_RDONLY, 0, false, false, NULL, j->mmap, NULL, &f);
40a46b
-        if (r < 0)
40a46b
-                return r;
40a46b
+        if (r < 0) {
40a46b
+                log_debug_errno(r, "Failed to open journal file %s: %m", path);
40a46b
+                goto fail;
40a46b
+        }
40a46b
 
40a46b
         /* journal_file_dump(f); */
40a46b
 
40a46b
         r = ordered_hashmap_put(j->files, f->path, f);
40a46b
         if (r < 0) {
40a46b
                 journal_file_close(f);
40a46b
-                return r;
40a46b
+                goto fail;
40a46b
         }
40a46b
 
40a46b
         log_debug("File %s added.", f->path);
40a46b
@@ -1227,10 +1232,17 @@ static int add_any_file(sd_journal *j, const char *path) {
40a46b
         j->current_invalidate_counter ++;
40a46b
 
40a46b
         return 0;
40a46b
+
40a46b
+fail:
40a46b
+        k = set_put_error(j, r);
40a46b
+        if (k < 0)
40a46b
+                return k;
40a46b
+
40a46b
+        return r;
40a46b
 }
40a46b
 
40a46b
 static int add_file(sd_journal *j, const char *prefix, const char *filename) {
40a46b
-        char *path = NULL;
40a46b
+        const char *path;
40a46b
 
40a46b
         assert(j);
40a46b
         assert(prefix);
40a46b
@@ -1250,24 +1262,20 @@ static int add_file(sd_journal *j, const char *prefix, const char *filename) {
40a46b
         return add_any_file(j, path);
40a46b
 }
40a46b
 
40a46b
-static int remove_file(sd_journal *j, const char *prefix, const char *filename) {
40a46b
-        _cleanup_free_ char *path;
40a46b
+static void remove_file(sd_journal *j, const char *prefix, const char *filename) {
40a46b
+        const char *path;
40a46b
         JournalFile *f;
40a46b
 
40a46b
         assert(j);
40a46b
         assert(prefix);
40a46b
         assert(filename);
40a46b
 
40a46b
-        path = strjoin(prefix, "/", filename, NULL);
40a46b
-        if (!path)
40a46b
-                return -ENOMEM;
40a46b
-
40a46b
+        path = strjoina(prefix, "/", filename);
40a46b
         f = ordered_hashmap_get(j->files, path);
40a46b
         if (!f)
40a46b
-                return 0;
40a46b
+                return;
40a46b
 
40a46b
         remove_file_real(j, f);
40a46b
-        return 0;
40a46b
 }
40a46b
 
40a46b
 static void remove_file_real(sd_journal *j, JournalFile *f) {
40a46b
@@ -1296,12 +1304,27 @@ static void remove_file_real(sd_journal *j, JournalFile *f) {
40a46b
         j->current_invalidate_counter ++;
40a46b
 }
40a46b
 
40a46b
+static int dirname_is_machine_id(const char *fn) {
40a46b
+        sd_id128_t id, machine;
40a46b
+        int r;
40a46b
+
40a46b
+        r = sd_id128_get_machine(&machine);
40a46b
+        if (r < 0)
40a46b
+                return r;
40a46b
+
40a46b
+        r = sd_id128_from_string(fn, &id;;
40a46b
+        if (r < 0)
40a46b
+                return r;
40a46b
+
40a46b
+        return sd_id128_equal(id, machine);
40a46b
+}
40a46b
+
40a46b
 static int add_directory(sd_journal *j, const char *prefix, const char *dirname) {
40a46b
         _cleanup_free_ char *path = NULL;
40a46b
-        int r;
40a46b
         _cleanup_closedir_ DIR *d = NULL;
40a46b
-        sd_id128_t id, mid;
40a46b
+        struct dirent *de = NULL;
40a46b
         Directory *m;
40a46b
+        int r, k;
40a46b
 
40a46b
         assert(j);
40a46b
         assert(prefix);
40a46b
@@ -1310,35 +1333,36 @@ static int add_directory(sd_journal *j, const char *prefix, const char *dirname)
40a46b
         log_debug("Considering %s/%s.", prefix, dirname);
40a46b
 
40a46b
         if ((j->flags & SD_JOURNAL_LOCAL_ONLY) &&
40a46b
-            (sd_id128_from_string(dirname, &id) < 0 ||
40a46b
-             sd_id128_get_machine(&mid) < 0 ||
40a46b
-             !(sd_id128_equal(id, mid) || path_startswith(prefix, "/run"))))
40a46b
+            !(dirname_is_machine_id(dirname) > 0 || path_startswith(prefix, "/run")))
40a46b
             return 0;
40a46b
 
40a46b
         path = strjoin(prefix, "/", dirname, NULL);
40a46b
-        if (!path)
40a46b
-                return -ENOMEM;
40a46b
+        if (!path) {
40a46b
+                r = -ENOMEM;
40a46b
+                goto fail;
40a46b
+        }
40a46b
 
40a46b
         d = opendir(path);
40a46b
         if (!d) {
40a46b
-                log_debug_errno(errno, "Failed to open %s: %m", path);
40a46b
-                if (errno == ENOENT)
40a46b
-                        return 0;
40a46b
-                return -errno;
40a46b
+                r = log_debug_errno(errno, "Failed to open directory %s: %m", path);
40a46b
+                goto fail;
40a46b
         }
40a46b
 
40a46b
         m = hashmap_get(j->directories_by_path, path);
40a46b
         if (!m) {
40a46b
                 m = new0(Directory, 1);
40a46b
-                if (!m)
40a46b
-                        return -ENOMEM;
40a46b
+                if (!m) {
40a46b
+                        r = -ENOMEM;
40a46b
+                        goto fail;
40a46b
+                }
40a46b
 
40a46b
                 m->is_root = false;
40a46b
                 m->path = path;
40a46b
 
40a46b
                 if (hashmap_put(j->directories_by_path, m->path, m) < 0) {
40a46b
                         free(m);
40a46b
-                        return -ENOMEM;
40a46b
+                        r = -ENOMEM;
40a46b
+                        goto fail;
40a46b
                 }
40a46b
 
40a46b
                 path = NULL; /* avoid freeing in cleanup */
40a46b
@@ -1360,41 +1384,30 @@ static int add_directory(sd_journal *j, const char *prefix, const char *dirname)
40a46b
                         inotify_rm_watch(j->inotify_fd, m->wd);
40a46b
         }
40a46b
 
40a46b
-        for (;;) {
40a46b
-                struct dirent *de;
40a46b
-
40a46b
-                errno = 0;
40a46b
-                de = readdir(d);
40a46b
-                if (!de && errno != 0) {
40a46b
-                        r = -errno;
40a46b
-                        log_debug_errno(errno, "Failed to read directory %s: %m", m->path);
40a46b
-                        return r;
40a46b
-                }
40a46b
-                if (!de)
40a46b
-                        break;
40a46b
+        FOREACH_DIRENT_ALL(de, d, return log_debug_errno(errno, "Failed to read directory %s: %m", m->path)) {
40a46b
 
40a46b
                 if (dirent_is_file_with_suffix(de, ".journal") ||
40a46b
-                    dirent_is_file_with_suffix(de, ".journal~")) {
40a46b
-                        r = add_file(j, m->path, de->d_name);
40a46b
-                        if (r < 0) {
40a46b
-                                log_debug_errno(r, "Failed to add file %s/%s: %m",
40a46b
-                                                m->path, de->d_name);
40a46b
-                                r = set_put_error(j, r);
40a46b
-                                if (r < 0)
40a46b
-                                        return r;
40a46b
-                        }
40a46b
-                }
40a46b
+                    dirent_is_file_with_suffix(de, ".journal~"))
40a46b
+                        (void) add_file(j, m->path, de->d_name);
40a46b
         }
40a46b
 
40a46b
         check_network(j, dirfd(d));
40a46b
 
40a46b
         return 0;
40a46b
+
40a46b
+fail:
40a46b
+        k = set_put_error(j, r);
40a46b
+        if (k < 0)
40a46b
+                return k;
40a46b
+
40a46b
+        return r;
40a46b
 }
40a46b
 
40a46b
-static int add_root_directory(sd_journal *j, const char *p) {
40a46b
+static int add_root_directory(sd_journal *j, const char *p, bool missing_ok) {
40a46b
         _cleanup_closedir_ DIR *d = NULL;
40a46b
+        struct dirent *de;
40a46b
         Directory *m;
40a46b
-        int r;
40a46b
+        int r, k;
40a46b
 
40a46b
         assert(j);
40a46b
         assert(p);
40a46b
@@ -1407,26 +1420,35 @@ static int add_root_directory(sd_journal *j, const char *p) {
40a46b
                 p = strjoina(j->prefix, p);
40a46b
 
40a46b
         d = opendir(p);
40a46b
-        if (!d)
40a46b
-                return -errno;
40a46b
+        if (!d) {
40a46b
+                if (errno == ENOENT && missing_ok)
40a46b
+                        return 0;
40a46b
+
40a46b
+                r = log_debug_errno(errno, "Failed to open root directory %s: %m", p);
40a46b
+                goto fail;
40a46b
+        }
40a46b
 
40a46b
         m = hashmap_get(j->directories_by_path, p);
40a46b
         if (!m) {
40a46b
                 m = new0(Directory, 1);
40a46b
-                if (!m)
40a46b
-                        return -ENOMEM;
40a46b
+                if (!m) {
40a46b
+                        r = -ENOMEM;
40a46b
+                        goto fail;
40a46b
+                }
40a46b
 
40a46b
                 m->is_root = true;
40a46b
                 m->path = strdup(p);
40a46b
                 if (!m->path) {
40a46b
                         free(m);
40a46b
-                        return -ENOMEM;
40a46b
+                        r = -ENOMEM;
40a46b
+                        goto fail;
40a46b
                 }
40a46b
 
40a46b
                 if (hashmap_put(j->directories_by_path, m->path, m) < 0) {
40a46b
                         free(m->path);
40a46b
                         free(m);
40a46b
-                        return -ENOMEM;
40a46b
+                        r = -ENOMEM;
40a46b
+                        goto fail;
40a46b
                 }
40a46b
 
40a46b
                 j->current_invalidate_counter ++;
40a46b
@@ -1449,42 +1471,27 @@ static int add_root_directory(sd_journal *j, const char *p) {
40a46b
         if (j->no_new_files)
40a46b
                 return 0;
40a46b
 
40a46b
-        for (;;) {
40a46b
-                struct dirent *de;
40a46b
+        FOREACH_DIRENT_ALL(de, d, return log_debug_errno(errno, "Failed to read directory %s: %m", m->path)) {
40a46b
                 sd_id128_t id;
40a46b
 
40a46b
-                errno = 0;
40a46b
-                de = readdir(d);
40a46b
-                if (!de && errno != 0) {
40a46b
-                        r = -errno;
40a46b
-                        log_debug_errno(errno, "Failed to read directory %s: %m", m->path);
40a46b
-                        return r;
40a46b
-                }
40a46b
-                if (!de)
40a46b
-                        break;
40a46b
-
40a46b
                 if (dirent_is_file_with_suffix(de, ".journal") ||
40a46b
-                    dirent_is_file_with_suffix(de, ".journal~")) {
40a46b
-                        r = add_file(j, m->path, de->d_name);
40a46b
-                        if (r < 0) {
40a46b
-                                log_debug_errno(r, "Failed to add file %s/%s: %m",
40a46b
-                                                m->path, de->d_name);
40a46b
-                                r = set_put_error(j, r);
40a46b
-                                if (r < 0)
40a46b
-                                        return r;
40a46b
-                        }
40a46b
-                } else if ((de->d_type == DT_DIR || de->d_type == DT_LNK || de->d_type == DT_UNKNOWN) &&
40a46b
-                           sd_id128_from_string(de->d_name, &id) >= 0) {
40a46b
-
40a46b
-                        r = add_directory(j, m->path, de->d_name);
40a46b
-                        if (r < 0)
40a46b
-                                log_debug_errno(r, "Failed to add directory %s/%s: %m", m->path, de->d_name);
40a46b
-                }
40a46b
+                    dirent_is_file_with_suffix(de, ".journal~"))
40a46b
+                        (void) add_file(j, m->path, de->d_name);
40a46b
+                else if (IN_SET(de->d_type, DT_DIR, DT_LNK, DT_UNKNOWN) &&
40a46b
+                         sd_id128_from_string(de->d_name, &id) >= 0)
40a46b
+                        (void) add_directory(j, m->path, de->d_name);
40a46b
         }
40a46b
 
40a46b
         check_network(j, dirfd(d));
40a46b
 
40a46b
         return 0;
40a46b
+
40a46b
+fail:
40a46b
+        k = set_put_error(j, r);
40a46b
+        if (k < 0)
40a46b
+                return k;
40a46b
+
40a46b
+        return r;
40a46b
 }
40a46b
 
40a46b
 static void remove_directory(sd_journal *j, Directory *d) {
40a46b
@@ -1509,8 +1516,8 @@ static void remove_directory(sd_journal *j, Directory *d) {
40a46b
 }
40a46b
 
40a46b
 static int add_search_paths(sd_journal *j) {
40a46b
-        int r;
40a46b
-        const char search_paths[] =
40a46b
+
40a46b
+        static const char search_paths[] =
40a46b
                 "/run/log/journal\0"
40a46b
                 "/var/log/journal\0";
40a46b
         const char *p;
40a46b
@@ -1520,14 +1527,8 @@ static int add_search_paths(sd_journal *j) {
40a46b
         /* We ignore most errors here, since the idea is to only open
40a46b
          * what's actually accessible, and ignore the rest. */
40a46b
 
40a46b
-        NULSTR_FOREACH(p, search_paths) {
40a46b
-                r = add_root_directory(j, p);
40a46b
-                if (r < 0 && r != -ENOENT) {
40a46b
-                        r = set_put_error(j, r);
40a46b
-                        if (r < 0)
40a46b
-                                return r;
40a46b
-                }
40a46b
-        }
40a46b
+        NULSTR_FOREACH(p, search_paths)
40a46b
+                (void) add_root_directory(j, p, true);
40a46b
 
40a46b
         return 0;
40a46b
 }
40a46b
@@ -1551,17 +1552,14 @@ static int add_current_paths(sd_journal *j) {
40a46b
                 if (!dir)
40a46b
                         return -ENOMEM;
40a46b
 
40a46b
-                r = add_root_directory(j, dir);
40a46b
-                if (r < 0) {
40a46b
-                        set_put_error(j, r);
40a46b
+                r = add_root_directory(j, dir, true);
40a46b
+                if (r < 0)
40a46b
                         return r;
40a46b
-                }
40a46b
         }
40a46b
 
40a46b
         return 0;
40a46b
 }
40a46b
 
40a46b
-
40a46b
 static int allocate_inotify(sd_journal *j) {
40a46b
         assert(j);
40a46b
 
40a46b
@@ -1689,11 +1687,9 @@ _public_ int sd_journal_open_directory(sd_journal **ret, const char *path, int f
40a46b
         if (!j)
40a46b
                 return -ENOMEM;
40a46b
 
40a46b
-        r = add_root_directory(j, path);
40a46b
-        if (r < 0) {
40a46b
-                set_put_error(j, r);
40a46b
+        r = add_root_directory(j, path, false);
40a46b
+        if (r < 0)
40a46b
                 goto fail;
40a46b
-        }
40a46b
 
40a46b
         *ret = j;
40a46b
         return 0;
40a46b
@@ -1718,10 +1714,8 @@ _public_ int sd_journal_open_files(sd_journal **ret, const char **paths, int fla
40a46b
 
40a46b
         STRV_FOREACH(path, paths) {
40a46b
                 r = add_any_file(j, *path);
40a46b
-                if (r < 0) {
40a46b
-                        log_error_errno(r, "Failed to open %s: %m", *path);
40a46b
+                if (r < 0)
40a46b
                         goto fail;
40a46b
-                }
40a46b
         }
40a46b
 
40a46b
         j->no_new_files = true;
40a46b
@@ -2061,7 +2055,7 @@ _public_ int sd_journal_get_fd(sd_journal *j) {
40a46b
         if (j->no_new_files)
40a46b
                 r = add_current_paths(j);
40a46b
         else if (j->path)
40a46b
-                r = add_root_directory(j, j->path);
40a46b
+                r = add_root_directory(j, j->path, true);
40a46b
         else
40a46b
                 r = add_search_paths(j);
40a46b
         if (r < 0)
40a46b
@@ -2108,7 +2102,6 @@ _public_ int sd_journal_get_timeout(sd_journal *j, uint64_t *timeout_usec) {
40a46b
 
40a46b
 static void process_inotify_event(sd_journal *j, struct inotify_event *e) {
40a46b
         Directory *d;
40a46b
-        int r;
40a46b
 
40a46b
         assert(j);
40a46b
         assert(e);
40a46b
@@ -2124,20 +2117,10 @@ static void process_inotify_event(sd_journal *j, struct inotify_event *e) {
40a46b
 
40a46b
                         /* Event for a journal file */
40a46b
 
40a46b
-                        if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB)) {
40a46b
-                                r = add_file(j, d->path, e->name);
40a46b
-                                if (r < 0) {
40a46b
-                                        log_debug_errno(r, "Failed to add file %s/%s: %m",
40a46b
-                                                        d->path, e->name);
40a46b
-                                        set_put_error(j, r);
40a46b
-                                }
40a46b
-
40a46b
-                        } else if (e->mask & (IN_DELETE|IN_MOVED_FROM|IN_UNMOUNT)) {
40a46b
-
40a46b
-                                r = remove_file(j, d->path, e->name);
40a46b
-                                if (r < 0)
40a46b
-                                        log_debug_errno(r, "Failed to remove file %s/%s: %m", d->path, e->name);
40a46b
-                        }
40a46b
+                        if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB))
40a46b
+                                (void) add_file(j, d->path, e->name);
40a46b
+                        else if (e->mask & (IN_DELETE|IN_MOVED_FROM|IN_UNMOUNT))
40a46b
+                                remove_file(j, d->path, e->name);
40a46b
 
40a46b
                 } else if (!d->is_root && e->len == 0) {
40a46b
 
40a46b
@@ -2150,11 +2133,8 @@ static void process_inotify_event(sd_journal *j, struct inotify_event *e) {
40a46b
 
40a46b
                         /* Event for root directory */
40a46b
 
40a46b
-                        if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB)) {
40a46b
-                                r = add_directory(j, d->path, e->name);
40a46b
-                                if (r < 0)
40a46b
-                                        log_debug_errno(r, "Failed to add directory %s/%s: %m", d->path, e->name);
40a46b
-                        }
40a46b
+                        if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB))
40a46b
+                                (void) add_directory(j, d->path, e->name);
40a46b
                 }
40a46b
 
40a46b
                 return;
40a46b
@@ -2163,7 +2143,7 @@ static void process_inotify_event(sd_journal *j, struct inotify_event *e) {
40a46b
         if (e->mask & IN_IGNORED)
40a46b
                 return;
40a46b
 
40a46b
-        log_warning("Unknown inotify event.");
40a46b
+        log_debug("Unknown inotify event.");
40a46b
 }
40a46b
 
40a46b
 static int determine_change(sd_journal *j) {