sd-journal: various clean-ups and modernizations

- Always print a debug log message about files and directories we cannot
  open right when it happens instead of the caller, thus reducing the
  number of places where we need to generate the debug message.

- Always push the errors we encounter immediately into the error set,
  when we run into them, instead of in the caller. Thus, we never forget
  to push them in.

- Use stack instead of heap memory where we can.

- Make remove_file() void, since it cannot fail anyway and always
  returned 0.

- Make local machine check of journal directories explicit in a
  function, to make things more readable.

- Port to all directory listing loops FOREACH_DIRENT_ALL()

- sd-daemon is library code, hence never log at higher log levels than
  LOG_DEBUG.

1 files changed, 113 insertions, 140 deletions

diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
index 218df8cebe..b5e1d05fa6 100644
--- a/src/journal/sd-journal.c
+++ b/src/journal/sd-journal.c
@@ -1183,6 +1183,8 @@ static bool file_has_type_prefix(const char *prefix, const char *filename) {
 }
 
 static bool file_type_wanted(int flags, const char *filename) {
+        assert(filename);
+
         if (!endswith(filename, ".journal") && !endswith(filename, ".journal~"))
                 return false;
 
@@ -1207,7 +1209,7 @@ static bool file_type_wanted(int flags, const char *filename) {
 
 static int add_any_file(sd_journal *j, const char *path) {
         JournalFile *f = NULL;
-        int r;
+        int r, k;
 
         assert(j);
         assert(path);
@@ -1216,20 +1218,23 @@ static int add_any_file(sd_journal *j, const char *path) {
                 return 0;
 
         if (ordered_hashmap_size(j->files) >= JOURNAL_FILES_MAX) {
-                log_warning("Too many open journal files, not adding %s.", path);
-                return set_put_error(j, -ETOOMANYREFS);
+                log_debug("Too many open journal files, not adding %s.", path);
+                r = -ETOOMANYREFS;
+                goto fail;
         }
 
         r = journal_file_open(path, O_RDONLY, 0, false, false, NULL, j->mmap, NULL, &f);
-        if (r < 0)
-                return r;
+        if (r < 0) {
+                log_debug_errno(r, "Failed to open journal file %s: %m", path);
+                goto fail;
+        }
 
         /* journal_file_dump(f); */
 
         r = ordered_hashmap_put(j->files, f->path, f);
         if (r < 0) {
                 journal_file_close(f);
-                return r;
+                goto fail;
         }
 
         log_debug("File %s added.", f->path);
@@ -1239,11 +1244,17 @@ static int add_any_file(sd_journal *j, const char *path) {
         j->current_invalidate_counter ++;
 
         return 0;
+
+fail:
+        k = set_put_error(j, r);
+        if (k < 0)
+                return k;
+
+        return r;
 }
 
 static int add_file(sd_journal *j, const char *prefix, const char *filename) {
-        _cleanup_free_ char *path = NULL;
-        int r;
+        const char *path;
 
         assert(j);
         assert(prefix);
@@ -1253,34 +1264,24 @@ static int add_file(sd_journal *j, const char *prefix, const char *filename) {
             !file_type_wanted(j->flags, filename))
                 return 0;
 
-        path = strjoin(prefix, "/", filename, NULL);
-        if (!path)
-                return -ENOMEM;
-
-        r = add_any_file(j, path);
-        if (r == -ENOENT)
-                return 0;
-        return r;
+        path = strjoina(prefix, "/", filename);
+        return add_any_file(j, path);
 }
 
-static int remove_file(sd_journal *j, const char *prefix, const char *filename) {
-        _cleanup_free_ char *path;
+static void remove_file(sd_journal *j, const char *prefix, const char *filename) {
+        const char *path;
         JournalFile *f;
 
         assert(j);
         assert(prefix);
         assert(filename);
 
-        path = strjoin(prefix, "/", filename, NULL);
-        if (!path)
-                return -ENOMEM;
-
+        path = strjoina(prefix, "/", filename);
         f = ordered_hashmap_get(j->files, path);
         if (!f)
-                return 0;
+                return;
 
         remove_file_real(j, f);
-        return 0;
 }
 
 static void remove_file_real(sd_journal *j, JournalFile *f) {
@@ -1309,12 +1310,27 @@ static void remove_file_real(sd_journal *j, JournalFile *f) {
         j->current_invalidate_counter ++;
 }
 
+static int dirname_is_machine_id(const char *fn) {
+        sd_id128_t id, machine;
+        int r;
+
+        r = sd_id128_get_machine(&machine);
+        if (r < 0)
+                return r;
+
+        r = sd_id128_from_string(fn, &id);
+        if (r < 0)
+                return r;
+
+        return sd_id128_equal(id, machine);
+}
+
 static int add_directory(sd_journal *j, const char *prefix, const char *dirname) {
         _cleanup_free_ char *path = NULL;
-        int r;
         _cleanup_closedir_ DIR *d = NULL;
-        sd_id128_t id, mid;
+        struct dirent *de = NULL;
         Directory *m;
+        int r, k;
 
         assert(j);
         assert(prefix);
@@ -1323,35 +1339,36 @@ static int add_directory(sd_journal *j, const char *prefix, const char *dirname)
         log_debug("Considering %s/%s.", prefix, dirname);
 
         if ((j->flags & SD_JOURNAL_LOCAL_ONLY) &&
-            (sd_id128_from_string(dirname, &id) < 0 ||
-             sd_id128_get_machine(&mid) < 0 ||
-             !(sd_id128_equal(id, mid) || path_startswith(prefix, "/run"))))
+            !(dirname_is_machine_id(dirname) > 0 || path_startswith(prefix, "/run")))
             return 0;
 
         path = strjoin(prefix, "/", dirname, NULL);
-        if (!path)
-                return -ENOMEM;
+        if (!path) {
+                r = -ENOMEM;
+                goto fail;
+        }
 
         d = opendir(path);
         if (!d) {
-                log_debug_errno(errno, "Failed to open %s: %m", path);
-                if (errno == ENOENT)
-                        return 0;
-                return -errno;
+                r = log_debug_errno(errno, "Failed to open directory %s: %m", path);
+                goto fail;
         }
 
         m = hashmap_get(j->directories_by_path, path);
         if (!m) {
                 m = new0(Directory, 1);
-                if (!m)
-                        return -ENOMEM;
+                if (!m) {
+                        r = -ENOMEM;
+                        goto fail;
+                }
 
                 m->is_root = false;
                 m->path = path;
 
                 if (hashmap_put(j->directories_by_path, m->path, m) < 0) {
                         free(m);
-                        return -ENOMEM;
+                        r = -ENOMEM;
+                        goto fail;
                 }
 
                 path = NULL; /* avoid freeing in cleanup */
@@ -1373,41 +1390,30 @@ static int add_directory(sd_journal *j, const char *prefix, const char *dirname)
                         inotify_rm_watch(j->inotify_fd, m->wd);
         }
 
-        for (;;) {
-                struct dirent *de;
-
-                errno = 0;
-                de = readdir(d);
-                if (!de && errno != 0) {
-                        r = -errno;
-                        log_debug_errno(errno, "Failed to read directory %s: %m", m->path);
-                        return r;
-                }
-                if (!de)
-                        break;
+        FOREACH_DIRENT_ALL(de, d, return log_debug_errno(errno, "Failed to read directory %s: %m", m->path)) {
 
                 if (dirent_is_file_with_suffix(de, ".journal") ||
-                    dirent_is_file_with_suffix(de, ".journal~")) {
-                        r = add_file(j, m->path, de->d_name);
-                        if (r < 0) {
-                                log_debug_errno(r, "Failed to add file %s/%s: %m",
-                                                m->path, de->d_name);
-                                r = set_put_error(j, r);
-                                if (r < 0)
-                                        return r;
-                        }
-                }
+                    dirent_is_file_with_suffix(de, ".journal~"))
+                        (void) add_file(j, m->path, de->d_name);
         }
 
         check_network(j, dirfd(d));
 
         return 0;
+
+fail:
+        k = set_put_error(j, r);
+        if (k < 0)
+                return k;
+
+        return r;
 }
 
-static int add_root_directory(sd_journal *j, const char *p) {
+static int add_root_directory(sd_journal *j, const char *p, bool missing_ok) {
         _cleanup_closedir_ DIR *d = NULL;
+        struct dirent *de;
         Directory *m;
-        int r;
+        int r, k;
 
         assert(j);
         assert(p);
@@ -1420,26 +1426,35 @@ static int add_root_directory(sd_journal *j, const char *p) {
                 p = strjoina(j->prefix, p);
 
         d = opendir(p);
-        if (!d)
-                return -errno;
+        if (!d) {
+                if (errno == ENOENT && missing_ok)
+                        return 0;
+
+                r = log_debug_errno(errno, "Failed to open root directory %s: %m", p);
+                goto fail;
+        }
 
         m = hashmap_get(j->directories_by_path, p);
         if (!m) {
                 m = new0(Directory, 1);
-                if (!m)
-                        return -ENOMEM;
+                if (!m) {
+                        r = -ENOMEM;
+                        goto fail;
+                }
 
                 m->is_root = true;
                 m->path = strdup(p);
                 if (!m->path) {
                         free(m);
-                        return -ENOMEM;
+                        r = -ENOMEM;
+                        goto fail;
                 }
 
                 if (hashmap_put(j->directories_by_path, m->path, m) < 0) {
                         free(m->path);
                         free(m);
-                        return -ENOMEM;
+                        r = -ENOMEM;
+                        goto fail;
                 }
 
                 j->current_invalidate_counter ++;
@@ -1462,42 +1477,27 @@ static int add_root_directory(sd_journal *j, const char *p) {
         if (j->no_new_files)
                 return 0;
 
-        for (;;) {
-                struct dirent *de;
+        FOREACH_DIRENT_ALL(de, d, return log_debug_errno(errno, "Failed to read directory %s: %m", m->path)) {
                 sd_id128_t id;
 
-                errno = 0;
-                de = readdir(d);
-                if (!de && errno != 0) {
-                        r = -errno;
-                        log_debug_errno(errno, "Failed to read directory %s: %m", m->path);
-                        return r;
-                }
-                if (!de)
-                        break;
-
                 if (dirent_is_file_with_suffix(de, ".journal") ||
-                    dirent_is_file_with_suffix(de, ".journal~")) {
-                        r = add_file(j, m->path, de->d_name);
-                        if (r < 0) {
-                                log_debug_errno(r, "Failed to add file %s/%s: %m",
-                                                m->path, de->d_name);
-                                r = set_put_error(j, r);
-                                if (r < 0)
-                                        return r;
-                        }
-                } else if ((de->d_type == DT_DIR || de->d_type == DT_LNK || de->d_type == DT_UNKNOWN) &&
-                           sd_id128_from_string(de->d_name, &id) >= 0) {
-
-                        r = add_directory(j, m->path, de->d_name);
-                        if (r < 0)
-                                log_debug_errno(r, "Failed to add directory %s/%s: %m", m->path, de->d_name);
-                }
+                    dirent_is_file_with_suffix(de, ".journal~"))
+                        (void) add_file(j, m->path, de->d_name);
+                else if (IN_SET(de->d_type, DT_DIR, DT_LNK, DT_UNKNOWN) &&
+                         sd_id128_from_string(de->d_name, &id) >= 0)
+                        (void) add_directory(j, m->path, de->d_name);
         }
 
         check_network(j, dirfd(d));
 
         return 0;
+
+fail:
+        k = set_put_error(j, r);
+        if (k < 0)
+                return k;
+
+        return r;
 }
 
 static void remove_directory(sd_journal *j, Directory *d) {
@@ -1522,8 +1522,8 @@ static void remove_directory(sd_journal *j, Directory *d) {
 }
 
 static int add_search_paths(sd_journal *j) {
-        int r;
-        const char search_paths[] =
+
+        static const char search_paths[] =
                 "/run/log/journal\0"
                 "/var/log/journal\0";
         const char *p;
@@ -1533,14 +1533,8 @@ static int add_search_paths(sd_journal *j) {
         /* We ignore most errors here, since the idea is to only open
          * what's actually accessible, and ignore the rest. */
 
-        NULSTR_FOREACH(p, search_paths) {
-                r = add_root_directory(j, p);
-                if (r < 0 && r != -ENOENT) {
-                        r = set_put_error(j, r);
-                        if (r < 0)
-                                return r;
-                }
-        }
+        NULSTR_FOREACH(p, search_paths)
+                (void) add_root_directory(j, p, true);
 
         return 0;
 }
@@ -1564,17 +1558,14 @@ static int add_current_paths(sd_journal *j) {
                 if (!dir)
                         return -ENOMEM;
 
-                r = add_root_directory(j, dir);
-                if (r < 0) {
-                        set_put_error(j, r);
+                r = add_root_directory(j, dir, true);
+                if (r < 0)
                         return r;
-                }
         }
 
         return 0;
 }
 
-
 static int allocate_inotify(sd_journal *j) {
         assert(j);
 
@@ -1702,11 +1693,9 @@ _public_ int sd_journal_open_directory(sd_journal **ret, const char *path, int f
         if (!j)
                 return -ENOMEM;
 
-        r = add_root_directory(j, path);
-        if (r < 0) {
-                set_put_error(j, r);
+        r = add_root_directory(j, path, false);
+        if (r < 0)
                 goto fail;
-        }
 
         *ret = j;
         return 0;
@@ -1731,10 +1720,8 @@ _public_ int sd_journal_open_files(sd_journal **ret, const char **paths, int fla
 
         STRV_FOREACH(path, paths) {
                 r = add_any_file(j, *path);
-                if (r < 0) {
-                        log_error_errno(r, "Failed to open %s: %m", *path);
+                if (r < 0)
                         goto fail;
-                }
         }
 
         j->no_new_files = true;
@@ -2074,7 +2061,7 @@ _public_ int sd_journal_get_fd(sd_journal *j) {
         if (j->no_new_files)
                 r = add_current_paths(j);
         else if (j->path)
-                r = add_root_directory(j, j->path);
+                r = add_root_directory(j, j->path, true);
         else
                 r = add_search_paths(j);
         if (r < 0)
@@ -2121,7 +2108,6 @@ _public_ int sd_journal_get_timeout(sd_journal *j, uint64_t *timeout_usec) {
 
 static void process_inotify_event(sd_journal *j, struct inotify_event *e) {
         Directory *d;
-        int r;
 
         assert(j);
         assert(e);
@@ -2137,20 +2123,10 @@ static void process_inotify_event(sd_journal *j, struct inotify_event *e) {
 
                         /* Event for a journal file */
 
-                        if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB)) {
-                                r = add_file(j, d->path, e->name);
-                                if (r < 0) {
-                                        log_debug_errno(r, "Failed to add file %s/%s: %m",
-                                                        d->path, e->name);
-                                        set_put_error(j, r);
-                                }
-
-                        } else if (e->mask & (IN_DELETE|IN_MOVED_FROM|IN_UNMOUNT)) {
-
-                                r = remove_file(j, d->path, e->name);
-                                if (r < 0)
-                                        log_debug_errno(r, "Failed to remove file %s/%s: %m", d->path, e->name);
-                        }
+                        if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB))
+                                (void) add_file(j, d->path, e->name);
+                        else if (e->mask & (IN_DELETE|IN_MOVED_FROM|IN_UNMOUNT))
+                                remove_file(j, d->path, e->name);
 
                 } else if (!d->is_root && e->len == 0) {
 
@@ -2163,11 +2139,8 @@ static void process_inotify_event(sd_journal *j, struct inotify_event *e) {
 
                         /* Event for root directory */
 
-                        if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB)) {
-                                r = add_directory(j, d->path, e->name);
-                                if (r < 0)
-                                        log_debug_errno(r, "Failed to add directory %s/%s: %m", d->path, e->name);
-                        }
+                        if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB))
+                                (void) add_directory(j, d->path, e->name);
                 }
 
                 return;
@@ -2176,7 +2149,7 @@ static void process_inotify_event(sd_journal *j, struct inotify_event *e) {
         if (e->mask & IN_IGNORED)
                 return;
 
-        log_warning("Unknown inotify event.");
+        log_debug("Unknown inotify event.");
 }
 
 static int determine_change(sd_journal *j) {