From 76cbafcdd4451c27c058c4c8ada454caf0eb3ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 17 Jul 2020 21:00:12 +0200 Subject: [PATCH] sd-journal: when enumerating, continue even after an inaccessible field SD_JOURNAL_FOREACH_DATA() and SD_JOURNAL_FOREACH_UNIQUE() would immediately terminate when a field couldn't be accessed. This can happen for example when a field is compressed with an unavailable compression format. But it's likely that this is the wrong thing to do: the caller for example might want to iterate over the fields but isn't interested in all of them. coredumpctl is like this: it uses SD_JOURNAL_FOREACH_DATA() but only uses a subset of the fields. Add two new functions sd_journal_enumerate_good_data() and sd_journal_enumerate_good_unique() that retry sd_journal_enumerate_data() and sd_journal_enumerate_unique() if the return value is something that applies to a single field: ENOBUS, E2BIG, EOPNOTSUPP. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1856037. An alternative would be to make the macros themselves smarter instead of adding new symbols, and do the looping internally in the macro. I don't like that approach for two reasons. First, it would embed the logic in the macro, so recompilation would be required if we decide to update the logic. With the current version of the patch, recompilation is required to use the new symbols, but after that, library upgrades are enough. So the current approach is safer in case further updates are needed. Second, our headers use primitive C, and it is hard to do the macros without using newer features. --- src/journal/journal-internal.h | 9 +++++++++ src/journal/sd-journal.c | 27 +++++++++++++++++++++++++++ src/libsystemd/libsystemd.sym | 3 +++ src/systemd/sd-journal.h | 10 ++++++---- 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/journal/journal-internal.h b/src/journal/journal-internal.h index a649acf634..d87b0a11e5 100644 --- a/src/journal/journal-internal.h +++ b/src/journal/journal-internal.h @@ -127,3 +127,12 @@ void journal_print_header(sd_journal *j); #define JOURNAL_FOREACH_DATA_RETVAL(j, data, l, retval) \ for (sd_journal_restart_data(j); ((retval) = sd_journal_enumerate_data((j), &(data), &(l))) > 0; ) + +/* All errors that we might encounter while extracting a field that are not real errors, + * but only mean that the field is too large or we don't support the compression. */ +static inline bool JOURNAL_ERRNO_IS_UNAVAILABLE_FIELD(int r) { + return IN_SET(abs(r), + ENOBUFS, /* Field or decompressed field too large */ + E2BIG, /* Field too large for pointer width */ + EPROTONOSUPPORT); /* Unsupported compression */ +} diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index 853dd0c28b..6fb0abb419 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -2462,6 +2462,19 @@ _public_ int sd_journal_enumerate_data(sd_journal *j, const void **data, size_t return 1; } +_public_ int sd_journal_enumerate_available_data(sd_journal *j, const void **data, size_t *size) { + for (;;) { + int r; + + r = sd_journal_enumerate_data(j, data, size); + if (r >= 0) + return r; + if (!JOURNAL_ERRNO_IS_UNAVAILABLE_FIELD(r)) + return r; + j->current_field++; /* Try with the next field */ + } +} + _public_ void sd_journal_restart_data(sd_journal *j) { if (!j) return; @@ -3002,6 +3015,20 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_ } } +_public_ int sd_journal_enumerate_available_unique(sd_journal *j, const void **data, size_t *size) { + for (;;) { + int r; + + r = sd_journal_enumerate_unique(j, data, size); + if (r >= 0) + return r; + if (!JOURNAL_ERRNO_IS_UNAVAILABLE_FIELD(r)) + return r; + /* Try with the next field. sd_journal_enumerate_unique() modifies state, so on the next try + * we will access the next field. */ + } +} + _public_ void sd_journal_restart_unique(sd_journal *j) { if (!j) return; diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 6f00985f5d..1e654b49ea 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -717,4 +717,7 @@ global: sd_path_lookup_strv; sd_notify_barrier; + + sd_journal_enumerate_available_data; + sd_journal_enumerate_available_unique; } LIBSYSTEMD_245; diff --git a/src/systemd/sd-journal.h b/src/systemd/sd-journal.h index a232992f03..d220f21aa2 100644 --- a/src/systemd/sd-journal.h +++ b/src/systemd/sd-journal.h @@ -105,6 +105,7 @@ int sd_journal_get_data_threshold(sd_journal *j, size_t *sz); int sd_journal_get_data(sd_journal *j, const char *field, const void **data, size_t *l); int sd_journal_enumerate_data(sd_journal *j, const void **data, size_t *l); +int sd_journal_enumerate_available_data(sd_journal *j, const void **data, size_t *l); void sd_journal_restart_data(sd_journal *j); int sd_journal_add_match(sd_journal *j, const void *data, size_t size); @@ -128,6 +129,7 @@ int sd_journal_get_usage(sd_journal *j, uint64_t *bytes); int sd_journal_query_unique(sd_journal *j, const char *field); int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_t *l); +int sd_journal_enumerate_available_unique(sd_journal *j, const void **data, size_t *l); void sd_journal_restart_unique(sd_journal *j); int sd_journal_enumerate_fields(sd_journal *j, const char **field); @@ -156,13 +158,13 @@ int sd_journal_has_persistent_files(sd_journal *j); if (sd_journal_seek_tail(j) < 0) { } \ else while (sd_journal_previous(j) > 0) -/* Iterate through the data fields of the current journal entry */ +/* Iterate through all available data fields of the current journal entry */ #define SD_JOURNAL_FOREACH_DATA(j, data, l) \ - for (sd_journal_restart_data(j); sd_journal_enumerate_data((j), &(data), &(l)) > 0; ) + for (sd_journal_restart_data(j); sd_journal_enumerate_available_data((j), &(data), &(l)) > 0; ) -/* Iterate through the all known values of a specific field */ +/* Iterate through all available values of a specific field */ #define SD_JOURNAL_FOREACH_UNIQUE(j, data, l) \ - for (sd_journal_restart_unique(j); sd_journal_enumerate_unique((j), &(data), &(l)) > 0; ) + for (sd_journal_restart_unique(j); sd_journal_enumerate_available_unique((j), &(data), &(l)) > 0; ) /* Iterate through all known field names */ #define SD_JOURNAL_FOREACH_FIELD(j, field) \