[systemd-commits] 7 commits - src/journal src/shared

Lennart Poettering lennart at kemper.freedesktop.org
Mon May 18 15:38:08 PDT 2015


 src/journal/journalctl.c |  401 ++++++++++++++++++++++++++++-------------------
 src/shared/logs-show.c   |   56 +++---
 2 files changed, 273 insertions(+), 184 deletions(-)

New commits:
commit 9530e0d023b0e9308f19eadf6e42cdc25bc30793
Author: Lennart Poettering <lennart at poettering.net>
Date:   Tue May 19 00:35:02 2015 +0200

    journalctl: unify how we free boot id lists a bit
    
    Instead of use LIST_FOREACH_SAFE, just use the same, seperate destructor
    everywhere.

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index bbc4258..76ec082 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -852,6 +852,15 @@ static int add_matches(sd_journal *j, char **args) {
         return 0;
 }
 
+static void boot_id_free_all(BootId *l) {
+
+        while (l) {
+                BootId *i = l;
+                LIST_REMOVE(boot_list, l, i);
+                free(i);
+        }
+}
+
 static int discover_next_boot(
                 sd_journal *j,
                 BootId **boot,
@@ -929,6 +938,7 @@ static int discover_next_boot(
 
         *boot = next_boot;
         next_boot = NULL;
+
         return 0;
 }
 
@@ -1000,9 +1010,7 @@ static int get_boots(
 
                 r = discover_next_boot(j, &current, advance_older, !query_ref_boot);
                 if (r < 0) {
-                        BootId *id, *id_next;
-                        LIST_FOREACH_SAFE(boot_list, id, id_next, head)
-                                free(id);
+                        boot_id_free_all(head);
                         return r;
                 }
 
@@ -1038,7 +1046,7 @@ finish:
 
 static int list_boots(sd_journal *j) {
         int w, i, count;
-        BootId *id, *id_next, *all_ids;
+        BootId *id, *all_ids;
 
         assert(j);
 
@@ -1054,7 +1062,7 @@ static int list_boots(sd_journal *j) {
         w = DECIMAL_STR_WIDTH(count - 1) + 1;
 
         i = 0;
-        LIST_FOREACH_SAFE(boot_list, id, id_next, all_ids) {
+        LIST_FOREACH(boot_list, id, all_ids) {
                 char a[FORMAT_TIMESTAMP_MAX], b[FORMAT_TIMESTAMP_MAX];
 
                 printf("% *i " SD_ID128_FORMAT_STR " %s—%s\n",
@@ -1063,9 +1071,10 @@ static int list_boots(sd_journal *j) {
                        format_timestamp_maybe_utc(a, sizeof(a), id->first),
                        format_timestamp_maybe_utc(b, sizeof(b), id->last));
                 i++;
-                free(id);
         }
 
+        boot_id_free_all(all_ids);
+
         return 0;
 }
 

commit b56d608e6977f7731a6fccd593c50eec1a69f466
Author: Lennart Poettering <lennart at poettering.net>
Date:   Tue May 19 00:25:45 2015 +0200

    journalctl: clean up how we log errors
    
    All functions should either log the errors they run into, or only return
    them in which case the caller should log them.
    
    Make sure this rule is followed, so that each error is logged precisely
    once, and neither never, nor more than once.

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index c9a259a..bbc4258 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -818,17 +818,11 @@ static int add_matches(sd_journal *j, char **args) {
                                         }
                                 } else
                                         t = strappend("_EXE=", path);
-                        } else if (S_ISCHR(st.st_mode)) {
-                                if (asprintf(&t, "_KERNEL_DEVICE=c%u:%u",
-                                             major(st.st_rdev),
-                                             minor(st.st_rdev)) < 0)
-                                        return -ENOMEM;
-                        } else if (S_ISBLK(st.st_mode)) {
-                                if (asprintf(&t, "_KERNEL_DEVICE=b%u:%u",
-                                             major(st.st_rdev),
-                                             minor(st.st_rdev)) < 0)
-                                        return -ENOMEM;
-                        } else {
+                        } else if (S_ISCHR(st.st_mode))
+                                (void) asprintf(&t, "_KERNEL_DEVICE=c%u:%u", major(st.st_rdev), minor(st.st_rdev));
+                        else if (S_ISBLK(st.st_mode))
+                                (void) asprintf(&t, "_KERNEL_DEVICE=b%u:%u", major(st.st_rdev), minor(st.st_rdev));
+                        else {
                                 log_error("File is neither a device node, nor regular file, nor executable: %s", *i);
                                 return -EINVAL;
                         }
@@ -893,7 +887,7 @@ static int discover_next_boot(
 
         next_boot = new0(BootId, 1);
         if (!next_boot)
-                return log_oom();
+                return -ENOMEM;
 
         r = sd_journal_get_monotonic_usec(j, NULL, &next_boot->id);
         if (r < 0)
@@ -1049,7 +1043,9 @@ static int list_boots(sd_journal *j) {
         assert(j);
 
         count = get_boots(j, &all_ids, NULL, 0);
-        if (count <= 0)
+        if (count < 0)
+                return log_error_errno(count, "Failed to determine boots: %m");
+        if (count == 0)
                 return count;
 
         pager_open_if_enabled();
@@ -1109,7 +1105,7 @@ static int add_boot(sd_journal *j) {
 
         r = sd_journal_add_conjunction(j);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to add conjunction: %m");
 
         return 0;
 }
@@ -1127,22 +1123,24 @@ static int add_dmesg(sd_journal *j) {
 
         r = sd_journal_add_conjunction(j);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to add conjunction: %m");
 
         return 0;
 }
 
-static int get_possible_units(sd_journal *j,
-                              const char *fields,
-                              char **patterns,
-                              Set **units) {
+static int get_possible_units(
+                sd_journal *j,
+                const char *fields,
+                char **patterns,
+                Set **units) {
+
         _cleanup_set_free_free_ Set *found;
         const char *field;
         int r;
 
         found = set_new(&string_hash_ops);
         if (!found)
-                return log_oom();
+                return -ENOMEM;
 
         NULSTR_FOREACH(field, fields) {
                 const void *data;
@@ -1165,7 +1163,7 @@ static int get_possible_units(sd_journal *j,
 
                         u = strndup((char*) data + prefix, size - prefix);
                         if (!u)
-                                return log_oom();
+                                return -ENOMEM;
 
                         STRV_FOREACH(pattern, patterns)
                                 if (fnmatch(*pattern, u, FNM_NOESCAPE) == 0) {
@@ -1329,7 +1327,7 @@ static int add_priorities(sd_journal *j) {
 
         r = sd_journal_add_conjunction(j);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to add conjunction: %m");
 
         return 0;
 }
@@ -1930,21 +1928,20 @@ int main(int argc, char *argv[]) {
         }
 
         r = add_priorities(j);
-        if (r < 0) {
-                log_error_errno(r, "Failed to add filter for priorities: %m");
+        if (r < 0)
                 goto finish;
-        }
 
         r = add_matches(j, argv + optind);
-        if (r < 0) {
-                log_error_errno(r, "Failed to add filters: %m");
+        if (r < 0)
                 goto finish;
-        }
 
         if (_unlikely_(log_get_max_level() >= LOG_DEBUG)) {
                 _cleanup_free_ char *filter;
 
                 filter = journal_make_match_string(j);
+                if (!filter)
+                        return log_oom();
+
                 log_debug("Journal filter: %s", filter);
         }
 
@@ -1954,7 +1951,7 @@ int main(int argc, char *argv[]) {
 
                 r = sd_journal_set_data_threshold(j, 0);
                 if (r < 0) {
-                        log_error("Failed to unset data size threshold");
+                        log_error_errno(r, "Failed to unset data size threshold: %m");
                         goto finish;
                 }
 
@@ -1986,8 +1983,10 @@ int main(int argc, char *argv[]) {
         /* Opening the fd now means the first sd_journal_wait() will actually wait */
         if (arg_follow) {
                 r = sd_journal_get_fd(j);
-                if (r < 0)
+                if (r < 0) {
+                        log_error_errno(r, "Failed to get journal fd: %m");
                         goto finish;
+                }
         }
 
         if (arg_cursor || arg_after_cursor) {
diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c
index 54179e9..ac5eb16 100644
--- a/src/shared/logs-show.c
+++ b/src/shared/logs-show.c
@@ -272,7 +272,7 @@ static int output_short(
         }
 
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to get journal fields: %m");
 
         if (!message)
                 return 0;
@@ -408,11 +408,9 @@ static int output_verbose(
         r = sd_journal_get_data(j, "_SOURCE_REALTIME_TIMESTAMP", &data, &length);
         if (r == -ENOENT)
                 log_debug("Source realtime timestamp not found");
-        else if (r < 0) {
-                log_full_errno(r == -EADDRNOTAVAIL ? LOG_DEBUG : LOG_ERR, r,
-                               "Failed to get source realtime timestamp: %m");
-                return r;
-        } else {
+        else if (r < 0)
+                return log_full_errno(r == -EADDRNOTAVAIL ? LOG_DEBUG : LOG_ERR, r, "Failed to get source realtime timestamp: %m");
+        else {
                 _cleanup_free_ char *value = NULL;
                 size_t size;
 
@@ -428,11 +426,8 @@ static int output_verbose(
 
         if (r < 0) {
                 r = sd_journal_get_realtime_usec(j, &realtime);
-                if (r < 0) {
-                        log_full_errno(r == -EADDRNOTAVAIL ? LOG_DEBUG : LOG_ERR, r,
-                                       "Failed to get realtime timestamp: %m");
-                        return r;
-                }
+                if (r < 0)
+                        return log_full_errno(r == -EADDRNOTAVAIL ? LOG_DEBUG : LOG_ERR, r, "Failed to get realtime timestamp: %m");
         }
 
         r = sd_journal_get_cursor(j, &cursor);
@@ -682,7 +677,7 @@ static int output_json(
 
         h = hashmap_new(&string_hash_ops);
         if (!h)
-                return -ENOMEM;
+                return log_oom();
 
         /* First round, iterate through the entry and count how often each field appears */
         JOURNAL_FOREACH_DATA_RETVAL(j, data, length, r) {
@@ -700,7 +695,7 @@ static int output_json(
 
                 n = strndup(data, eq - (const char*) data);
                 if (!n) {
-                        r = -ENOMEM;
+                        r = log_oom();
                         goto finish;
                 }
 
@@ -709,13 +704,16 @@ static int output_json(
                         r = hashmap_put(h, n, UINT_TO_PTR(1));
                         if (r < 0) {
                                 free(n);
+                                log_oom();
                                 goto finish;
                         }
                 } else {
                         r = hashmap_update(h, n, UINT_TO_PTR(u + 1));
                         free(n);
-                        if (r < 0)
+                        if (r < 0) {
+                                log_oom();
                                 goto finish;
+                        }
                 }
         }
 
@@ -753,7 +751,7 @@ static int output_json(
 
                         n = strndup(data, m);
                         if (!n) {
-                                r = -ENOMEM;
+                                r = log_oom();
                                 goto finish;
                         }
 
@@ -948,11 +946,11 @@ static int show_journal(FILE *f,
         /* Seek to end */
         r = sd_journal_seek_tail(j);
         if (r < 0)
-                goto finish;
+                return log_error_errno(r, "Failed to seek to tail: %m");
 
         r = sd_journal_previous_skip(j, how_many);
         if (r < 0)
-                goto finish;
+                return log_error_errno(r, "Failed to skip previous: %m");
 
         for (;;) {
                 for (;;) {
@@ -961,7 +959,7 @@ static int show_journal(FILE *f,
                         if (need_seek) {
                                 r = sd_journal_next(j);
                                 if (r < 0)
-                                        goto finish;
+                                        return log_error_errno(r, "Failed to iterate through journal: %m");
                         }
 
                         if (r == 0)
@@ -977,7 +975,7 @@ static int show_journal(FILE *f,
                                 if (r == -ESTALE)
                                         continue;
                                 else if (r < 0)
-                                        goto finish;
+                                        return log_error_errno(r, "Failed to get journal time: %m");
 
                                 if (usec < not_before)
                                         continue;
@@ -988,7 +986,7 @@ static int show_journal(FILE *f,
 
                         r = output_journal(f, j, mode, n_columns, flags, ellipsized);
                         if (r < 0)
-                                goto finish;
+                                return r;
                 }
 
                 if (warn_cutoff && line < how_many && not_before > 0) {
@@ -999,11 +997,11 @@ static int show_journal(FILE *f,
 
                         r = sd_id128_get_boot(&boot_id);
                         if (r < 0)
-                                goto finish;
+                                return log_error_errno(r, "Failed to get boot id: %m");
 
                         r = sd_journal_get_cutoff_monotonic_usec(j, boot_id, &cutoff, NULL);
                         if (r < 0)
-                                goto finish;
+                                return log_error_errno(r, "Failed to get journal cutoff time: %m");
 
                         if (r > 0 && not_before < cutoff) {
                                 maybe_print_begin_newline(f, &flags);
@@ -1018,12 +1016,11 @@ static int show_journal(FILE *f,
 
                 r = sd_journal_wait(j, USEC_INFINITY);
                 if (r < 0)
-                        goto finish;
+                        return log_error_errno(r, "Failed to wait for journal: %m");
 
         }
 
-finish:
-        return r;
+        return 0;
 }
 
 int add_matches_for_unit(sd_journal *j, const char *unit) {
@@ -1220,7 +1217,7 @@ int add_match_this_boot(sd_journal *j, const char *machine) {
 
         r = sd_journal_add_conjunction(j);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to add conjunction: %m");
 
         return 0;
 }
@@ -1250,7 +1247,7 @@ int show_journal_by_unit(
 
         r = sd_journal_open(&j, journal_open_flags);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to open journal: %m");
 
         r = add_match_this_boot(j, NULL);
         if (r < 0)
@@ -1261,12 +1258,15 @@ int show_journal_by_unit(
         else
                 r = add_matches_for_user_unit(j, unit, uid);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to add unit matches: %m");
 
         if (_unlikely_(log_get_max_level() >= LOG_DEBUG)) {
                 _cleanup_free_ char *filter;
 
                 filter = journal_make_match_string(j);
+                if (!filter)
+                        return log_oom();
+
                 log_debug("Journal filter: %s", filter);
         }
 

commit 45bc27b621c51b9d0e0229835deb6d188bcd417b
Author: Lennart Poettering <lennart at poettering.net>
Date:   Tue May 19 00:24:27 2015 +0200

    journalctl: rename boot_id_t to BootId
    
    So far we tried to reserve the _t suffix to types we use like a value in
    contrast to types we use as objects, hence let's do this in journalctl
    too.

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index d0cc5d9..c9a259a 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -125,12 +125,12 @@ static enum {
         ACTION_VACUUM,
 } arg_action = ACTION_SHOW;
 
-typedef struct boot_id_t {
+typedef struct BootId {
         sd_id128_t id;
         uint64_t first;
         uint64_t last;
-        LIST_FIELDS(struct boot_id_t, boot_list);
-} boot_id_t;
+        LIST_FIELDS(struct BootId, boot_list);
+} BootId;
 
 static void pager_open_if_enabled(void) {
 
@@ -858,13 +858,15 @@ static int add_matches(sd_journal *j, char **args) {
         return 0;
 }
 
-static int discover_next_boot(sd_journal *j,
-                              boot_id_t **boot,
-                              bool advance_older,
-                              bool read_realtime) {
+static int discover_next_boot(
+                sd_journal *j,
+                BootId **boot,
+                bool advance_older,
+                bool read_realtime) {
+
         int r;
         char match[9+32+1] = "_BOOT_ID=";
-        _cleanup_free_ boot_id_t *next_boot = NULL;
+        _cleanup_free_ BootId *next_boot = NULL;
 
         assert(j);
         assert(boot);
@@ -889,7 +891,7 @@ static int discover_next_boot(sd_journal *j,
         else if (r == 0)
                 return 0; /* End of journal, yay. */
 
-        next_boot = new0(boot_id_t, 1);
+        next_boot = new0(BootId, 1);
         if (!next_boot)
                 return log_oom();
 
@@ -936,13 +938,15 @@ static int discover_next_boot(sd_journal *j,
         return 0;
 }
 
-static int get_boots(sd_journal *j,
-                     boot_id_t **boots,
-                     boot_id_t *query_ref_boot,
-                     int ref_boot_offset) {
+static int get_boots(
+                sd_journal *j,
+                BootId **boots,
+                BootId *query_ref_boot,
+                int ref_boot_offset) {
+
         bool skip_once;
         int r, count = 0;
-        boot_id_t *head = NULL, *tail = NULL;
+        BootId *head = NULL, *tail = NULL;
         const bool advance_older = query_ref_boot && ref_boot_offset <= 0;
 
         assert(j);
@@ -997,12 +1001,12 @@ static int get_boots(sd_journal *j,
                 /* No sd_journal_next/previous here. */
         }
 
-        while (true) {
-                _cleanup_free_ boot_id_t *current = NULL;
+        for (;;) {
+                _cleanup_free_ BootId *current = NULL;
 
                 r = discover_next_boot(j, &current, advance_older, !query_ref_boot);
                 if (r < 0) {
-                        boot_id_t *id, *id_next;
+                        BootId *id, *id_next;
                         LIST_FOREACH_SAFE(boot_list, id, id_next, head)
                                 free(id);
                         return r;
@@ -1040,7 +1044,7 @@ finish:
 
 static int list_boots(sd_journal *j) {
         int w, i, count;
-        boot_id_t *id, *id_next, *all_ids;
+        BootId *id, *id_next, *all_ids;
 
         assert(j);
 
@@ -1072,7 +1076,7 @@ static int list_boots(sd_journal *j) {
 static int add_boot(sd_journal *j) {
         char match[9+32+1] = "_BOOT_ID=";
         int r;
-        boot_id_t ref_boot_id = {};
+        BootId ref_boot_id = {};
 
         assert(j);
 

commit 26b9f165c3ebf513110e3de88c92fcbf3011271d
Author: Lennart Poettering <lennart at poettering.net>
Date:   Tue May 19 00:22:56 2015 +0200

    journalctl: lstat() should suffice if we call canonicalize_file_name() first

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 3a59d7f..d0cc5d9 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -795,7 +795,7 @@ static int add_matches(sd_journal *j, char **args) {
                         p = canonicalize_file_name(*i);
                         path = p ? p : *i;
 
-                        if (stat(path, &st) < 0)
+                        if (lstat(path, &st) < 0)
                                 return log_error_errno(errno, "Couldn't stat file: %m");
 
                         if (S_ISREG(st.st_mode) && (0111 & st.st_mode)) {

commit d52da2057f06c49d50ed99300dc407c0227b1a32
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon May 18 23:54:05 2015 +0200

    journalctl: free all command line argument objects
    
    let's try to be valgrind clean

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index fa2be3b..3a59d7f 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -37,7 +37,6 @@
 
 #include "sd-journal.h"
 #include "sd-bus.h"
-
 #include "log.h"
 #include "logs-show.h"
 #include "util.h"
@@ -1915,8 +1914,6 @@ int main(int argc, char *argv[]) {
                 goto finish;
 
         r = add_units(j);
-        arg_system_units = strv_free(arg_system_units);
-        arg_user_units = strv_free(arg_user_units);
         if (r < 0) {
                 log_error_errno(r, "Failed to add filter for units: %m");
                 goto finish;
@@ -2183,5 +2180,9 @@ finish:
 
         strv_free(arg_file);
 
+        strv_free(arg_syslog_identifier);
+        strv_free(arg_system_units);
+        strv_free(arg_user_units);
+
         return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
 }

commit 909dea0c7ced0042fa3acd8cd05f5007a2cf2cea
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon May 18 23:50:34 2015 +0200

    journalctl: only have a single exit path from main()
    
    That way we can be sure we execute the destructors properly, and can be
    valgrind-clean.

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index b51df7d..fa2be3b 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -452,7 +452,7 @@ static int parse_argv(int argc, char *argv[]) {
                         arg_boot = true;
 
                         if (optarg) {
-                                r =  parse_boot_descriptor(optarg, &arg_boot_id, &arg_boot_offset);
+                                r = parse_boot_descriptor(optarg, &arg_boot_id, &arg_boot_offset);
                                 if (r < 0) {
                                         log_error("Failed to parse boot descriptor '%s'", optarg);
                                         return -EINVAL;
@@ -1848,12 +1848,12 @@ int main(int argc, char *argv[]) {
         if (r < 0) {
                 log_error_errno(r, "Failed to open %s: %m",
                                 arg_directory ? arg_directory : arg_file ? "files" : "journal");
-                return EXIT_FAILURE;
+                goto finish;
         }
 
         r = access_check(j);
         if (r < 0)
-                return EXIT_FAILURE;
+                goto finish;
 
         if (arg_action == ACTION_VERIFY) {
                 r = verify(j);
@@ -1862,7 +1862,8 @@ int main(int argc, char *argv[]) {
 
         if (arg_action == ACTION_PRINT_HEADER) {
                 journal_print_header(j);
-                return EXIT_SUCCESS;
+                r = 0;
+                goto finish;
         }
 
         if (arg_action == ACTION_DISK_USAGE) {
@@ -1871,11 +1872,11 @@ int main(int argc, char *argv[]) {
 
                 r = sd_journal_get_usage(j, &bytes);
                 if (r < 0)
-                        return EXIT_FAILURE;
+                        goto finish;
 
                 printf("Archived and active journals take up %s on disk.\n",
                        format_bytes(sbytes, sizeof(sbytes), bytes));
-                return EXIT_SUCCESS;
+                goto finish;
         }
 
         if (arg_action == ACTION_VACUUM) {
@@ -1895,7 +1896,7 @@ int main(int argc, char *argv[]) {
                         }
                 }
 
-                return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
+                goto finish;
         }
 
         if (arg_action == ACTION_LIST_BOOTS) {
@@ -1907,36 +1908,36 @@ int main(int argc, char *argv[]) {
          * It may need to seek the journal to find parent boot IDs. */
         r = add_boot(j);
         if (r < 0)
-                return EXIT_FAILURE;
+                goto finish;
 
         r = add_dmesg(j);
         if (r < 0)
-                return EXIT_FAILURE;
+                goto finish;
 
         r = add_units(j);
         arg_system_units = strv_free(arg_system_units);
         arg_user_units = strv_free(arg_user_units);
         if (r < 0) {
                 log_error_errno(r, "Failed to add filter for units: %m");
-                return EXIT_FAILURE;
+                goto finish;
         }
 
         r = add_syslog_identifier(j);
         if (r < 0) {
                 log_error_errno(r, "Failed to add filter for syslog identifiers: %m");
-                return EXIT_FAILURE;
+                goto finish;
         }
 
         r = add_priorities(j);
         if (r < 0) {
                 log_error_errno(r, "Failed to add filter for priorities: %m");
-                return EXIT_FAILURE;
+                goto finish;
         }
 
         r = add_matches(j, argv + optind);
         if (r < 0) {
                 log_error_errno(r, "Failed to add filters: %m");
-                return EXIT_FAILURE;
+                goto finish;
         }
 
         if (_unlikely_(log_get_max_level() >= LOG_DEBUG)) {
@@ -1953,13 +1954,13 @@ int main(int argc, char *argv[]) {
                 r = sd_journal_set_data_threshold(j, 0);
                 if (r < 0) {
                         log_error("Failed to unset data size threshold");
-                        return EXIT_FAILURE;
+                        goto finish;
                 }
 
                 r = sd_journal_query_unique(j, arg_field);
                 if (r < 0) {
                         log_error_errno(r, "Failed to query unique data objects: %m");
-                        return EXIT_FAILURE;
+                        goto finish;
                 }
 
                 SD_JOURNAL_FOREACH_UNIQUE(j, data, size) {
@@ -1977,22 +1978,24 @@ int main(int argc, char *argv[]) {
                         n_shown ++;
                 }
 
-                return EXIT_SUCCESS;
+                r = 0;
+                goto finish;
         }
 
         /* Opening the fd now means the first sd_journal_wait() will actually wait */
         if (arg_follow) {
                 r = sd_journal_get_fd(j);
                 if (r < 0)
-                        return EXIT_FAILURE;
+                        goto finish;
         }
 
         if (arg_cursor || arg_after_cursor) {
                 r = sd_journal_seek_cursor(j, arg_cursor ?: arg_after_cursor);
                 if (r < 0) {
                         log_error_errno(r, "Failed to seek to cursor: %m");
-                        return EXIT_FAILURE;
+                        goto finish;
                 }
+
                 if (!arg_reverse)
                         r = sd_journal_next_skip(j, 1 + !!arg_after_cursor);
                 else
@@ -2010,7 +2013,7 @@ int main(int argc, char *argv[]) {
                 r = sd_journal_seek_realtime_usec(j, arg_since);
                 if (r < 0) {
                         log_error_errno(r, "Failed to seek to date: %m");
-                        return EXIT_FAILURE;
+                        goto finish;
                 }
                 r = sd_journal_next(j);
 
@@ -2018,7 +2021,7 @@ int main(int argc, char *argv[]) {
                 r = sd_journal_seek_realtime_usec(j, arg_until);
                 if (r < 0) {
                         log_error_errno(r, "Failed to seek to date: %m");
-                        return EXIT_FAILURE;
+                        goto finish;
                 }
                 r = sd_journal_previous(j);
 
@@ -2026,7 +2029,7 @@ int main(int argc, char *argv[]) {
                 r = sd_journal_seek_tail(j);
                 if (r < 0) {
                         log_error_errno(r, "Failed to seek to tail: %m");
-                        return EXIT_FAILURE;
+                        goto finish;
                 }
 
                 r = sd_journal_previous_skip(j, arg_lines);
@@ -2035,7 +2038,7 @@ int main(int argc, char *argv[]) {
                 r = sd_journal_seek_tail(j);
                 if (r < 0) {
                         log_error_errno(r, "Failed to seek to tail: %m");
-                        return EXIT_FAILURE;
+                        goto finish;
                 }
 
                 r = sd_journal_previous(j);
@@ -2044,7 +2047,7 @@ int main(int argc, char *argv[]) {
                 r = sd_journal_seek_head(j);
                 if (r < 0) {
                         log_error_errno(r, "Failed to seek to head: %m");
-                        return EXIT_FAILURE;
+                        goto finish;
                 }
 
                 r = sd_journal_next(j);
@@ -2052,7 +2055,7 @@ int main(int argc, char *argv[]) {
 
         if (r < 0) {
                 log_error_errno(r, "Failed to iterate through journal: %m");
-                return EXIT_FAILURE;
+                goto finish;
         }
 
         if (!arg_follow)

commit 596a23293d28f93843aef86721b90043e74d3081
Author: Jan Janssen <medhefgo at web.de>
Date:   Fri May 1 15:15:16 2015 +0200

    journalctl: Improve boot ID lookup
    
    This method should greatly improve offset based lookup, by simply jumping
    from one boot to the next boot. It starts at the journal head to get the
    a boot ID, makes a _BOOT_ID match and then comes from the opposite
    journal direction (tail) to get to the end that boot. After flushing the matches
    and advancing the journal from that exact position, we arrive at the start
    of next boot. Rinse and repeat.
    
    This is faster than the old method of aggregating the full boot listing just
    so we can jump to a specific boot, which can be a real pain on big journals
    just for a mere "-b -1" case.
    
    As an additional benefit --list-boots should improve slightly too, because
    it does less seeking.
    
    Note that there can be a change in boot order with this lookup method
    because it will use the order of boots in the journal, not the realtime stamp
    stored in them. That's arguably better, though.
    Another deficiency is that it will get confused with boots interleaving in the
    journal, therefore, it will refuse operation in --merge, --file and --directory mode.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=72601

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 09f4e0f..b51df7d 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -130,6 +130,7 @@ typedef struct boot_id_t {
         sd_id128_t id;
         uint64_t first;
         uint64_t last;
+        LIST_FIELDS(struct boot_id_t, boot_list);
 } boot_id_t;
 
 static void pager_open_if_enabled(void) {
@@ -734,6 +735,11 @@ static int parse_argv(int argc, char *argv[]) {
                 return -EINVAL;
         }
 
+        if ((arg_boot || arg_action == ACTION_LIST_BOOTS) && (arg_file || arg_directory || arg_merge)) {
+                log_error("Using --boot or --list-boots with --file, --directory or --merge is not supported.");
+                return -EINVAL;
+        }
+
         return 1;
 }
 
@@ -853,111 +859,203 @@ static int add_matches(sd_journal *j, char **args) {
         return 0;
 }
 
-static int boot_id_cmp(const void *a, const void *b) {
-        uint64_t _a, _b;
+static int discover_next_boot(sd_journal *j,
+                              boot_id_t **boot,
+                              bool advance_older,
+                              bool read_realtime) {
+        int r;
+        char match[9+32+1] = "_BOOT_ID=";
+        _cleanup_free_ boot_id_t *next_boot = NULL;
 
-        _a = ((const boot_id_t *)a)->first;
-        _b = ((const boot_id_t *)b)->first;
+        assert(j);
+        assert(boot);
 
-        return _a < _b ? -1 : (_a > _b ? 1 : 0);
-}
+        /* We expect the journal to be on the last position of a boot
+         * (in relation to the direction we are going), so that the next
+         * invocation of sd_journal_next/previous will be from a different
+         * boot. We then collect any information we desire and then jump
+         * to the last location of the new boot by using a _BOOT_ID match
+         * coming from the other journal direction. */
 
-static int get_boots(sd_journal *j,
-                     boot_id_t **boots,
-                     unsigned int *count,
-                     boot_id_t *query_ref_boot) {
-        int r;
-        const void *data;
-        size_t length, allocated = 0;
+        /* Make sure we aren't restricted by any _BOOT_ID matches, so that
+         * we can actually advance to a *different* boot. */
+        sd_journal_flush_matches(j);
 
-        assert(j);
-        assert(boots);
-        assert(count);
+        if (advance_older)
+                r = sd_journal_previous(j);
+        else
+                r = sd_journal_next(j);
+        if (r < 0)
+                return r;
+        else if (r == 0)
+                return 0; /* End of journal, yay. */
+
+        next_boot = new0(boot_id_t, 1);
+        if (!next_boot)
+                return log_oom();
 
-        r = sd_journal_query_unique(j, "_BOOT_ID");
+        r = sd_journal_get_monotonic_usec(j, NULL, &next_boot->id);
         if (r < 0)
                 return r;
 
-        *count = 0;
-        SD_JOURNAL_FOREACH_UNIQUE(j, data, length) {
-                boot_id_t *id;
+        if (read_realtime) {
+                r = sd_journal_get_realtime_usec(j, &next_boot->first);
+                if (r < 0)
+                        return r;
+        }
 
-                assert(startswith(data, "_BOOT_ID="));
+        /* Now seek to the last occurrence of this boot ID. */
+        sd_id128_to_string(next_boot->id, match + 9);
+        r = sd_journal_add_match(j, match, sizeof(match) - 1);
+        if (r < 0)
+                return r;
 
-                if (!GREEDY_REALLOC(*boots, allocated, *count + 1))
-                        return log_oom();
+        if (advance_older)
+                r = sd_journal_seek_head(j);
+        else
+                r = sd_journal_seek_tail(j);
+        if (r < 0)
+                return r;
 
-                id = *boots + *count;
+        if (advance_older)
+                r = sd_journal_next(j);
+        else
+                r = sd_journal_previous(j);
+        if (r < 0)
+                return r;
+        else if (r == 0)
+                return -ENODATA; /* This shouldn't happen. We just came from this very boot ID. */
 
-                r = sd_id128_from_string(((const char *)data) + strlen("_BOOT_ID="), &id->id);
+        if (read_realtime) {
+                r = sd_journal_get_realtime_usec(j, &next_boot->last);
                 if (r < 0)
-                        continue;
+                        return r;
+        }
+
+        *boot = next_boot;
+        next_boot = NULL;
+        return 0;
+}
+
+static int get_boots(sd_journal *j,
+                     boot_id_t **boots,
+                     boot_id_t *query_ref_boot,
+                     int ref_boot_offset) {
+        bool skip_once;
+        int r, count = 0;
+        boot_id_t *head = NULL, *tail = NULL;
+        const bool advance_older = query_ref_boot && ref_boot_offset <= 0;
+
+        assert(j);
+
+        /* Adjust for the asymmetry that offset 0 is
+         * the last (and current) boot, while 1 is considered the
+         * (chronological) first boot in the journal. */
+        skip_once = query_ref_boot && sd_id128_is_null(query_ref_boot->id) && ref_boot_offset < 0;
+
+        /* Advance to the earliest/latest occurrence of our reference
+         * boot ID (taking our lookup direction into account), so that
+         * discover_next_boot() can do its job.
+         * If no reference is given, the journal head/tail will do,
+         * they're "virtual" boots after all. */
+        if (query_ref_boot && !sd_id128_is_null(query_ref_boot->id)) {
+                char match[9+32+1] = "_BOOT_ID=";
 
-                r = sd_journal_add_match(j, data, length);
+                sd_journal_flush_matches(j);
+
+                sd_id128_to_string(query_ref_boot->id, match + 9);
+                r = sd_journal_add_match(j, match, sizeof(match) - 1);
                 if (r < 0)
                         return r;
 
-                r = sd_journal_seek_head(j);
+                if (advance_older)
+                        r = sd_journal_seek_head(j);
+                else
+                        r = sd_journal_seek_tail(j);
                 if (r < 0)
                         return r;
 
-                r = sd_journal_next(j);
+                if (advance_older)
+                        r = sd_journal_next(j);
+                else
+                        r = sd_journal_previous(j);
                 if (r < 0)
                         return r;
                 else if (r == 0)
-                        goto flush;
-
-                r = sd_journal_get_realtime_usec(j, &id->first);
+                        goto finish;
+                else if (ref_boot_offset == 0) {
+                        count = 1;
+                        goto finish;
+                }
+        } else {
+                if (advance_older)
+                        r = sd_journal_seek_tail(j);
+                else
+                        r = sd_journal_seek_head(j);
                 if (r < 0)
                         return r;
 
-                if (query_ref_boot) {
-                        id->last = 0;
-                        if (sd_id128_equal(id->id, query_ref_boot->id))
-                                *query_ref_boot = *id;
-                } else {
-                        r = sd_journal_seek_tail(j);
-                        if (r < 0)
-                                return r;
+                /* No sd_journal_next/previous here. */
+        }
 
-                        r = sd_journal_previous(j);
-                        if (r < 0)
-                                return r;
-                        else if (r == 0)
-                                goto flush;
+        while (true) {
+                _cleanup_free_ boot_id_t *current = NULL;
 
-                        r = sd_journal_get_realtime_usec(j, &id->last);
-                        if (r < 0)
-                                return r;
+                r = discover_next_boot(j, &current, advance_older, !query_ref_boot);
+                if (r < 0) {
+                        boot_id_t *id, *id_next;
+                        LIST_FOREACH_SAFE(boot_list, id, id_next, head)
+                                free(id);
+                        return r;
                 }
 
-                (*count)++;
-        flush:
-                sd_journal_flush_matches(j);
+                if (!current)
+                        break;
+
+                if (query_ref_boot) {
+                        if (!skip_once)
+                                ref_boot_offset += advance_older ? 1 : -1;
+                        skip_once = false;
+
+                        if (ref_boot_offset == 0) {
+                                count = 1;
+                                query_ref_boot->id = current->id;
+                                break;
+                        }
+                } else {
+                        LIST_INSERT_AFTER(boot_list, head, tail, current);
+                        tail = current;
+                        current = NULL;
+                        count++;
+                }
         }
 
-        qsort_safe(*boots, *count, sizeof(boot_id_t), boot_id_cmp);
-        return 0;
+finish:
+        if (boots)
+                *boots = head;
+
+        sd_journal_flush_matches(j);
+
+        return count;
 }
 
 static int list_boots(sd_journal *j) {
-        int r, w, i;
-        unsigned int count;
-        boot_id_t *id;
-        _cleanup_free_ boot_id_t *all_ids = NULL;
+        int w, i, count;
+        boot_id_t *id, *id_next, *all_ids;
 
         assert(j);
 
-        r = get_boots(j, &all_ids, &count, NULL);
-        if (r < 0)
-                return r;
+        count = get_boots(j, &all_ids, NULL, 0);
+        if (count <= 0)
+                return count;
 
         pager_open_if_enabled();
 
         /* numbers are one less, but we need an extra char for the sign */
         w = DECIMAL_STR_WIDTH(count - 1) + 1;
 
-        for (id = all_ids, i = 0; id < all_ids + count; id++, i++) {
+        i = 0;
+        LIST_FOREACH_SAFE(boot_list, id, id_next, all_ids) {
                 char a[FORMAT_TIMESTAMP_MAX], b[FORMAT_TIMESTAMP_MAX];
 
                 printf("% *i " SD_ID128_FORMAT_STR " %s—%s\n",
@@ -965,39 +1063,8 @@ static int list_boots(sd_journal *j) {
                        SD_ID128_FORMAT_VAL(id->id),
                        format_timestamp_maybe_utc(a, sizeof(a), id->first),
                        format_timestamp_maybe_utc(b, sizeof(b), id->last));
-        }
-
-        return 0;
-}
-
-static int get_boot_id_by_offset(sd_journal *j, sd_id128_t *boot_id, int offset) {
-        int r;
-        unsigned int count;
-        boot_id_t ref_boot_id = {}, *id;
-        _cleanup_free_ boot_id_t *all_ids = NULL;
-
-        assert(j);
-        assert(boot_id);
-
-        ref_boot_id.id = *boot_id;
-        r = get_boots(j, &all_ids, &count, &ref_boot_id);
-        if (r < 0)
-                return r;
-
-        if (sd_id128_equal(*boot_id, SD_ID128_NULL)) {
-                if (offset > (int) count || offset <= -(int)count)
-                        return -EADDRNOTAVAIL;
-
-                *boot_id = all_ids[(offset <= 0)*count + offset - 1].id;
-        } else {
-                id = bsearch(&ref_boot_id, all_ids, count, sizeof(boot_id_t), boot_id_cmp);
-
-                if (!id ||
-                    offset <= 0 ? (id - all_ids) + offset < 0 :
-                                    (id - all_ids) + offset >= (int) count)
-                        return -EADDRNOTAVAIL;
-
-                *boot_id = (id + offset)->id;
+                i++;
+                free(id);
         }
 
         return 0;
@@ -1006,6 +1073,7 @@ static int get_boot_id_by_offset(sd_journal *j, sd_id128_t *boot_id, int offset)
 static int add_boot(sd_journal *j) {
         char match[9+32+1] = "_BOOT_ID=";
         int r;
+        boot_id_t ref_boot_id = {};
 
         assert(j);
 
@@ -1015,17 +1083,22 @@ static int add_boot(sd_journal *j) {
         if (arg_boot_offset == 0 && sd_id128_equal(arg_boot_id, SD_ID128_NULL))
                 return add_match_this_boot(j, arg_machine);
 
-        r = get_boot_id_by_offset(j, &arg_boot_id, arg_boot_offset);
-        if (r < 0) {
-                if (sd_id128_equal(arg_boot_id, SD_ID128_NULL))
-                        log_error_errno(r, "Failed to look up boot %+i: %m", arg_boot_offset);
+        ref_boot_id.id = arg_boot_id;
+        r = get_boots(j, NULL, &ref_boot_id, arg_boot_offset);
+        assert(r <= 1);
+        if (r <= 0) {
+                const char *reason = (r == 0) ? "No such boot ID in journal" : strerror(-r);
+
+                if (sd_id128_is_null(arg_boot_id))
+                        log_error("Failed to look up boot %+i: %s", arg_boot_offset, reason);
                 else
                         log_error("Failed to look up boot ID "SD_ID128_FORMAT_STR"%+i: %s",
-                                  SD_ID128_FORMAT_VAL(arg_boot_id), arg_boot_offset, strerror(-r));
-                return r;
+                                  SD_ID128_FORMAT_VAL(arg_boot_id), arg_boot_offset, reason);
+
+                return r == 0 ? -ENODATA : r;
         }
 
-        sd_id128_to_string(arg_boot_id, match + 9);
+        sd_id128_to_string(ref_boot_id.id, match + 9);
 
         r = sd_journal_add_match(j, match, sizeof(match) - 1);
         if (r < 0)



More information about the systemd-commits mailing list