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