SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

coredumpctll: avoid unnecessary heap copy and decompression for field existence checks (#41520)

Browse Source 
`print_list()` and `print_info()` used `RETRIEVE()` to `strndup()` the
entire
`COREDUMP` field into a heap-allocated string, only to check whether it
exists.
With `sd_journal_set_data_threshold(j, 0)` in `print_info()`, this
copies the
full coredump binary (potentially hundreds of MB) to heap just to print
"Storage: journal".

This PR:

1. Makes `sd_journal_get_data()` output parameters optional
(`NULL`-safe), so
   callers can do pure existence checks without receiving the data.
2. Short-circuits `maybe_decompress_payload()` after
`decompress_startswith()`
   succeeds when neither output pointer is requested, skipping full blob
   decompression for compressed journal entries.
3. Switches coredumpctl to pass `NULL, NULL` for the existence checks
instead
   of heap-copying via `RETRIEVE()`.
This commit is contained in:
Luca Boccassi
2026-04-08 21:34:47 +01:00
committed by GitHub
parent 1e551f0d96 836de2900b
commit c0b3b855b7
4 changed files with 30 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
man/sd_journal_get_data.xml
+8 -2
View File
@@ -83,8 +83,10 @@
<para><function>sd_journal_get_data()</function> gets the data object associated with a specific field
from the current journal entry. It takes four arguments: the journal context object, a string with the
field name to request, plus a pair of pointers to pointer/size variables where the data object and its
size shall be stored in. The field name should be an entry field name. Well-known field names are listed in
field name to request, plus a pair of optional pointers to pointer/size variables where the data object and
its size shall be stored in. Either pointer may be <constant>NULL</constant>, in which case the corresponding
value is not returned (this is supported since version 261). The field name should be an entry field name.
Well-known field names are listed in
<citerefentry><refentrytitle>systemd.journal-fields</refentrytitle><manvolnum>7</manvolnum></citerefentry>,
but any field can be specified. The returned data is in a read-only memory map and is only valid until
the next invocation of <function>sd_journal_get_data()</function>,
@@ -162,6 +164,10 @@
<term><constant>-EINVAL</constant></term>
<listitem><para>One of the required parameters is <constant>NULL</constant> or invalid.
For <function>sd_journal_get_data()</function>, only the journal context object and field name
are required non-<constant>NULL</constant>. For <function>sd_journal_enumerate_data()</function>
and <function>sd_journal_enumerate_available_data()</function>,
<parameter>ret_data</parameter> and <parameter>ret_size</parameter> are required as well.
</para>
<xi:include href="version-info.xml" xpointer="v246"/></listitem>
src/coredump/coredumpctl.c
+12 -8
View File
@@ -553,14 +553,14 @@ static int print_list(FILE* file, sd_journal *j, Table *t) {
_cleanup_free_ char
*mid = NULL, *pid = NULL, *uid = NULL, *gid = NULL,
*sgnl = NULL, *exe = NULL, *comm = NULL,
*filename = NULL, *truncated = NULL, *coredump = NULL;
*filename = NULL, *truncated = NULL;
const void *d;
size_t l;
usec_t ts;
int r, signal_as_int = 0;
const char *present = NULL, *color = NULL;
uint64_t size = UINT64_MAX;
bool normal_coredump;
bool normal_coredump, has_inline_coredump;
uid_t uid_as_int = UID_INVALID;
gid_t gid_as_int = GID_INVALID;
pid_t pid_as_int = 0;
@@ -579,9 +579,11 @@ static int print_list(FILE* file, sd_journal *j, Table *t) {
RETRIEVE(d, l, "COREDUMP_COMM", comm);
RETRIEVE(d, l, "COREDUMP_FILENAME", filename);
RETRIEVE(d, l, "COREDUMP_TRUNCATED", truncated);
RETRIEVE(d, l, "COREDUMP", coredump);
}
/* Check for an inline coredump without copying the (potentially large) payload to heap. */
has_inline_coredump = sd_journal_get_data(j, "COREDUMP", NULL, NULL) >= 0;
if (!pid || !uid || !gid || !sgnl || !comm) {
log_warning("Found a coredump entry without mandatory fields (PID=%s, UID=%s, GID=%s, SIGNAL=%s, COMM=%s), ignoring.",
strna(pid), strna(uid), strna(gid), strna(sgnl), strna(comm));
@@ -605,7 +607,7 @@ static int print_list(FILE* file, sd_journal *j, Table *t) {
return r;
analyze_coredump_file(filename, &present, &color, &size);
} else if (coredump)
} else if (has_inline_coredump)
present = "journal";
else if (normal_coredump) {
present = "none";
@@ -641,12 +643,12 @@ static int print_info(FILE *file, sd_journal *j, bool need_space) {
*boot_id = NULL, *machine_id = NULL, *hostname = NULL,
*slice = NULL, *cgroup = NULL, *owner_uid = NULL,
*message = NULL, *timestamp = NULL, *filename = NULL,
*truncated = NULL, *coredump = NULL,
*truncated = NULL,
*pkgmeta_name = NULL, *pkgmeta_version = NULL, *pkgmeta_json = NULL,
*tid = NULL, *thread_name = NULL;
const void *d;
size_t l;
bool normal_coredump;
bool normal_coredump, has_inline_coredump;
int r;
assert(file);
@@ -673,7 +675,6 @@ static int print_info(FILE *file, sd_journal *j, bool need_space) {
RETRIEVE(d, l, "COREDUMP_TIMESTAMP", timestamp);
RETRIEVE(d, l, "COREDUMP_FILENAME", filename);
RETRIEVE(d, l, "COREDUMP_TRUNCATED", truncated);
RETRIEVE(d, l, "COREDUMP", coredump);
RETRIEVE(d, l, "COREDUMP_PACKAGE_NAME", pkgmeta_name);
RETRIEVE(d, l, "COREDUMP_PACKAGE_VERSION", pkgmeta_version);
RETRIEVE(d, l, "COREDUMP_PACKAGE_JSON", pkgmeta_json);
@@ -684,6 +685,9 @@ static int print_info(FILE *file, sd_journal *j, bool need_space) {
RETRIEVE(d, l, "MESSAGE", message);
}
/* Check for an inline coredump without copying the (potentially large) payload to heap. */
has_inline_coredump = sd_journal_get_data(j, "COREDUMP", NULL, NULL) >= 0;
if (need_space)
fputs("\n", file);
@@ -821,7 +825,7 @@ static int print_info(FILE *file, sd_journal *j, bool need_space) {
if (size != UINT64_MAX)
fprintf(file, " Size on Disk: %s\n", FORMAT_BYTES(size));
} else if (coredump)
} else if (has_inline_coredump)
fprintf(file, " Storage: journal\n");
else
fprintf(file, " Storage: none\n");
src/libsystemd/sd-journal/journal-file.c
+4
View File
@@ -1965,6 +1965,10 @@ static int maybe_decompress_payload(
*ret_size = 0;
return 0;
}
/* Caller only wants to check field existence, skip full decompression */
if (!ret_data && !ret_size)
return 1;
}
r = decompress_blob(compression, payload, size, &f->compress_buffer, &rsize, 0);
src/libsystemd/sd-journal/sd-journal.c
+6 -5
View File
@@ -2822,8 +2822,6 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void **
assert_return(j, -EINVAL);
assert_return(!journal_origin_changed(j), -ECHILD);
assert_return(field, -EINVAL);
assert_return(ret_data, -EINVAL);
assert_return(ret_size, -EINVAL);
assert_return(field_is_valid(field), -EINVAL);
f = j->current_file;
@@ -2846,7 +2844,8 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void **
size_t l;
p = journal_file_entry_item_object_offset(f, o, i);
r = journal_file_data_payload(f, NULL, p, field, field_length, j->data_threshold, &d, &l);
r = journal_file_data_payload(f, NULL, p, field, field_length, j->data_threshold,
ret_data ? &d : NULL, ret_size ? &l : NULL);
if (r == 0)
continue;
if (IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) {
@@ -2856,8 +2855,10 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void **
if (r < 0)
return r;
*ret_data = d;
*ret_size = l;
if (ret_data)
*ret_data = d;
if (ret_size)
*ret_size = l;
return 0;
}