journalctl-filter: several cleanups for add_matches()

- split it into small pieces to reduce indentation,
- return earlier if no filter specified,
- use journal_add_matchf() and journal_add_match_pair(),
  chase_and_stat(), is_symlink(), and sd_device_new_from_devname(),
- replace sd_device_get_devname() + stat() with sd_device_get_devnum(),
- refuse to install filter by path with --root, --image, or --machine
  at least now, as previously it has not worked anyway.

diff --git a/src/journal/journalctl-filter.c b/src/journal/journalctl-filter.c
index f013c42470a2a84b495564e688bff80c4e6fae99..3129e776769281484792b0d0d8cdb9a40c8775b6 100644 (file)
--- a/src/journal/journalctl-filter.c
+++ b/src/journal/journalctl-filter.c
@@ -314,30 +314,50 @@ static int add_facilities(sd_journal *j) {
         return sd_journal_add_conjunction(j);
 }
 
+static int add_matches_for_executable(sd_journal *j, const char *path) {
+        _cleanup_free_ char *interpreter = NULL;
+        int r;
+
+        assert(j);
+        assert(path);
+
+        if (executable_is_script(path, &interpreter) > 0) {
+                _cleanup_free_ char *comm = NULL;
+
+                r = path_extract_filename(path, &comm);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to extract filename of '%s': %m", path);
+
+                r = journal_add_match_pair(j, "_COMM", strshorten(comm, TASK_COMM_LEN-1));
+                if (r < 0)
+                        return log_error_errno(r, "Failed to add match: %m");
+
+                /* Append _EXE only if the interpreter is not a link. Otherwise, it might be outdated often. */
+                path = is_symlink(interpreter) > 0 ? interpreter : NULL;
+        }
+
+        if (path) {
+                r = journal_add_match_pair(j, "_EXE", path);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to add match: %m");
+        }
+
+        return 0;
+}
+
 static int add_matches_for_device(sd_journal *j, const char *devpath) {
         _cleanup_(sd_device_unrefp) sd_device *device = NULL;
-        sd_device *d = NULL;
-        struct stat st;
         int r;
 
         assert(j);
         assert(devpath);
 
-        if (!path_startswith(devpath, "/dev/"))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Devpath does not start with /dev/");
-
-        if (stat(devpath, &st) < 0)
-                return log_error_errno(errno, "Couldn't stat file: %m");
-
-        r = sd_device_new_from_stat_rdev(&device, &st);
+        r = sd_device_new_from_devname(&device, devpath);
         if (r < 0)
-                return log_error_errno(r, "Failed to get device from devnum " DEVNUM_FORMAT_STR ": %m", DEVNUM_FORMAT_VAL(st.st_rdev));
+                return log_error_errno(r, "Failed to get device '%s': %m", devpath);
 
-        for (d = device; d; ) {
-                _cleanup_free_ char *match = NULL;
-                const char *subsys, *sysname, *devnode;
-                sd_device *parent;
+        for (sd_device *d = device; d; ) {
+                const char *subsys, *sysname;
 
                 r = sd_device_get_subsystem(d, &subsys);
                 if (r < 0)
@@ -347,35 +367,22 @@ static int add_matches_for_device(sd_journal *j, const char *devpath) {
                 if (r < 0)
                         goto get_parent;
 
-                match = strjoin("_KERNEL_DEVICE=+", subsys, ":", sysname);
-                if (!match)
-                        return log_oom();
-
-                r = sd_journal_add_match(j, match, 0);
+                r = journal_add_matchf(j, "_KERNEL_DEVICE=+%s:%s", subsys, sysname);
                 if (r < 0)
                         return log_error_errno(r, "Failed to add match: %m");
 
-                if (sd_device_get_devname(d, &devnode) >= 0) {
-                        _cleanup_free_ char *match1 = NULL;
-
-                        r = stat(devnode, &st);
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to stat() device node \"%s\": %m", devnode);
-
-                        r = asprintf(&match1, "_KERNEL_DEVICE=%c" DEVNUM_FORMAT_STR, S_ISBLK(st.st_mode) ? 'b' : 'c', DEVNUM_FORMAT_VAL(st.st_rdev));
-                        if (r < 0)
-                                return log_oom();
-
-                        r = sd_journal_add_match(j, match1, 0);
+                dev_t devnum;
+                if (sd_device_get_devnum(d, &devnum) >= 0) {
+                        r = journal_add_matchf(j, "_KERNEL_DEVICE=%c" DEVNUM_FORMAT_STR,
+                                               streq(subsys, "block") ? 'b' : 'c',
+                                               DEVNUM_FORMAT_VAL(devnum));
                         if (r < 0)
                                 return log_error_errno(r, "Failed to add match: %m");
                 }
 
 get_parent:
-                if (sd_device_get_parent(d, &parent) < 0)
+                if (sd_device_get_parent(d, &d) < 0)
                         break;
-
-                d = parent;
         }
 
         r = add_match_this_boot(j, arg_machine);
@@ -385,83 +392,66 @@ get_parent:
         return 0;
 }
 
+static int add_matches_for_path(sd_journal *j, const char *path) {
+        _cleanup_free_ char *p = NULL;
+        struct stat st;
+        int r;
+
+        assert(j);
+        assert(path);
+
+        if (arg_root || arg_machine)
+                return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                       "An extra path in match filter is currently not supported with --root, --image, or -M/--machine.");
+
+        r = chase_and_stat(path, NULL, 0, &p, &st);
+        if (r < 0)
+                return log_error_errno(r, "Couldn't canonicalize path '%s': %m", path);
+
+        if (S_ISREG(st.st_mode) && (0111 & st.st_mode))
+                return add_matches_for_executable(j, p);
+
+        if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))
+                return add_matches_for_device(j, p);
+
+        return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "File is neither a device node nor executable: %s", p);
+}
+
 static int add_matches(sd_journal *j, char **args) {
         bool have_term = false;
+        int r;
 
         assert(j);
 
-        STRV_FOREACH(i, args) {
-                int r;
+        if (strv_isempty(args))
+                return 0;
 
+        STRV_FOREACH(i, args)
                 if (streq(*i, "+")) {
                         if (!have_term)
                                 break;
+
                         r = sd_journal_add_disjunction(j);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to add disjunction: %m");
+
                         have_term = false;
 
                 } else if (path_is_absolute(*i)) {
-                        _cleanup_free_ char *p = NULL, *t = NULL, *t2 = NULL, *interpreter = NULL;
-                        struct stat st;
-
-                        r = chase(*i, NULL, CHASE_TRAIL_SLASH, &p, NULL);
+                        r = add_matches_for_path(j, *i);
                         if (r < 0)
-                                return log_error_errno(r, "Couldn't canonicalize path: %m");
-
-                        if (lstat(p, &st) < 0)
-                                return log_error_errno(errno, "Couldn't stat file: %m");
-
-                        if (S_ISREG(st.st_mode) && (0111 & st.st_mode)) {
-                                if (executable_is_script(p, &interpreter) > 0) {
-                                        _cleanup_free_ char *comm = NULL;
-
-                                        r = path_extract_filename(p, &comm);
-                                        if (r < 0)
-                                                return log_error_errno(r, "Failed to extract filename of '%s': %m", p);
-
-                                        t = strjoin("_COMM=", strshorten(comm, TASK_COMM_LEN-1));
-                                        if (!t)
-                                                return log_oom();
-
-                                        /* Append _EXE only if the interpreter is not a link.
-                                           Otherwise, it might be outdated often. */
-                                        if (lstat(interpreter, &st) == 0 && !S_ISLNK(st.st_mode)) {
-                                                t2 = strjoin("_EXE=", interpreter);
-                                                if (!t2)
-                                                        return log_oom();
-                                        }
-                                } else {
-                                        t = strjoin("_EXE=", p);
-                                        if (!t)
-                                                return log_oom();
-                                }
-
-                                r = sd_journal_add_match(j, t, 0);
-
-                                if (r >=0 && t2)
-                                        r = sd_journal_add_match(j, t2, 0);
-
-                        } else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-                                r = add_matches_for_device(j, p);
-                                if (r < 0)
-                                        return r;
-                        } else
-                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                                       "File is neither a device node, nor regular file, nor executable: %s",
-                                                       *i);
-
+                                return r;
                         have_term = true;
+
                 } else {
                         r = sd_journal_add_match(j, *i, 0);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to add match '%s': %m", *i);
                         have_term = true;
                 }
 
-                if (r < 0)
-                        return log_error_errno(r, "Failed to add match '%s': %m", *i);
-        }
-
-        if (!strv_isempty(args) && !have_term)
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "\"+\" can only be used between terms");
+        if (!have_term)
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "\"+\" can only be used between terms.");
 
         return 0;
 }