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.

diff --git a/src/journal/journal-internal.h b/src/journal/journal-internal.h
index a649acf634e74d4409da33af209cce716f5a7eaa..d87b0a11e5899dc55d49500554ba001ab06ea555 100644 (file)
--- 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 853dd0c28b7c92ef2808d5889f59200a3b67aca9..6fb0abb419d9e97f23083071537d813818c91c51 100644 (file)
--- 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 6f00985f5d8ae646187bb7a51dd97b5831ee4697..1e654b49ea3c1b07b10b87327bd993efa65a1917 100644 (file)
--- 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 a232992f0307b314cc63f36b369a9f97e4f7c4c2..d220f21aa29f518578d22d619f19aeefa250cfe8 100644 (file)
--- 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) \