From 54ed92e17c7aaa8149491d5f36a1e16890d79bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 23 Feb 2026 10:49:27 +0100 Subject: [PATCH 1/6] shared/format-table: use 'char*'-style in function signatures --- src/shared/format-table.c | 24 ++++++++++++------------ src/shared/format-table.h | 14 +++++++------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 8b5b88370f..18cebf8628 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -159,7 +159,7 @@ struct Table { bool *reverse_map; }; -Table *table_new_raw(size_t n_columns) { +Table* table_new_raw(size_t n_columns) { _cleanup_(table_unrefp) Table *t = NULL; assert(n_columns > 0); @@ -179,7 +179,7 @@ Table *table_new_raw(size_t n_columns) { return TAKE_PTR(t); } -Table *table_new_internal(const char *first_header, ...) { +Table* table_new_internal(const char *first_header, ...) { _cleanup_(table_unrefp) Table *t = NULL; size_t n_columns = 1; va_list ap; @@ -216,7 +216,7 @@ Table *table_new_internal(const char *first_header, ...) { return TAKE_PTR(t); } -Table *table_new_vertical(void) { +Table* table_new_vertical(void) { _cleanup_(table_unrefp) Table *t = NULL; TableCell *cell; @@ -242,7 +242,7 @@ Table *table_new_vertical(void) { return TAKE_PTR(t); } -static TableData *table_data_free(TableData *d) { +static TableData* table_data_free(TableData *d) { assert(d); free(d->formatted); @@ -260,7 +260,7 @@ static TableData *table_data_free(TableData *d) { DEFINE_PRIVATE_TRIVIAL_REF_UNREF_FUNC(TableData, table_data, table_data_free); DEFINE_TRIVIAL_CLEANUP_FUNC(TableData*, table_data_unref); -Table *table_unref(Table *t) { +Table* table_unref(Table *t) { if (!t) return NULL; @@ -425,7 +425,7 @@ static bool table_data_matches( return memcmp_safe(data, d->data, l) == 0; } -static TableData *table_data_new( +static TableData* table_data_new( TableDataType type, const void *data, size_t minimum_width, @@ -1602,7 +1602,7 @@ static char* format_strv_width(char **strv, size_t column_width) { return buf; } -static const char *table_data_format( +static const char* table_data_format( Table *t, TableData *d, bool avoid_uppercasing, @@ -2115,7 +2115,7 @@ static const char *table_data_format( return d->formatted; } -static const char *table_data_format_strip_ansi( +static const char* table_data_format_strip_ansi( Table *t, TableData *d, bool avoid_uppercasing, @@ -2252,7 +2252,7 @@ static int table_data_requested_width_height( return truncation_applied; } -static char *align_string_mem(const char *str, const char *url, size_t new_length, unsigned percent) { +static char* align_string_mem(const char *str, const char *url, size_t new_length, unsigned percent) { size_t w = 0, space, lspace, old_length, clickable_length; _cleanup_free_ char *clickable = NULL; const char *p; @@ -2851,7 +2851,7 @@ int table_set_reverse(Table *t, size_t column, bool b) { return 0; } -TableCell *table_get_cell(Table *t, size_t row, size_t column) { +TableCell* table_get_cell(Table *t, size_t row, size_t column) { size_t i; assert(t); @@ -2866,7 +2866,7 @@ TableCell *table_get_cell(Table *t, size_t row, size_t column) { return TABLE_INDEX_TO_CELL(i); } -const void *table_get(Table *t, TableCell *cell) { +const void* table_get(Table *t, TableCell *cell) { TableData *d; assert(t); @@ -3124,7 +3124,7 @@ static int table_make_json_field_name(Table *t, TableData *d, char **ret) { return 0; } -static const char *table_get_json_field_name(Table *t, size_t idx) { +static const char* table_get_json_field_name(Table *t, size_t idx) { assert(t); return idx < t->n_json_fields ? t->json_fields[idx] : NULL; diff --git a/src/shared/format-table.h b/src/shared/format-table.h index ec9989c54c..997ac20eb6 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -93,11 +93,11 @@ typedef enum TableErsatz { typedef struct Table Table; typedef struct TableCell TableCell; -Table *table_new_internal(const char *first_header, ...) _sentinel_; +Table* table_new_internal(const char *first_header, ...) _sentinel_; #define table_new(...) table_new_internal(__VA_ARGS__, NULL) -Table *table_new_raw(size_t n_columns); -Table *table_new_vertical(void); -Table *table_unref(Table *t); +Table* table_new_raw(size_t n_columns); +Table* table_new_vertical(void); +Table* table_unref(Table *t); DEFINE_TRIVIAL_CLEANUP_FUNC(Table*, table_unref); @@ -159,10 +159,10 @@ size_t table_get_columns(Table *t); size_t table_get_current_column(Table *t); -TableCell *table_get_cell(Table *t, size_t row, size_t column); +TableCell* table_get_cell(Table *t, size_t row, size_t column); -const void *table_get(Table *t, TableCell *cell); -const void *table_get_at(Table *t, size_t row, size_t column); +const void* table_get(Table *t, TableCell *cell); +const void* table_get_at(Table *t, size_t row, size_t column); int table_to_json(Table *t, sd_json_variant **ret); int table_print_json(Table *t, FILE *f, sd_json_format_flags_t json_flags); From 3abe1e53f657982a7a6ab6861cf0b3962cbbb3b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 3 Mar 2026 09:42:41 +0100 Subject: [PATCH 2/6] basic/allow-util: make free_many non-inline Definition of free_many is moved to the .c file, no particular reason for it to be inline and we can make the header file shorter. --- src/basic/alloc-util.c | 7 +++++++ src/basic/alloc-util.h | 7 +------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/basic/alloc-util.c b/src/basic/alloc-util.c index 58b70dede4..94102dd430 100644 --- a/src/basic/alloc-util.c +++ b/src/basic/alloc-util.c @@ -142,3 +142,10 @@ size_t malloc_sizeof_safe(void **xp) { assert_not_reached(); return sz; } + +void free_many(void **p, size_t n) { + assert(p || n == 0); + + FOREACH_ARRAY(i, p, n) + *i = mfree(*i); +} diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index cd062d5fe3..7d5c1da61f 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -206,12 +206,7 @@ size_t malloc_sizeof_safe(void **xp); VOID_0)) /* Free every element of the array. */ -static inline void free_many(void **p, size_t n) { - assert(p || n == 0); - - FOREACH_ARRAY(i, p, n) - *i = mfree(*i); -} +void free_many(void **p, size_t n); /* Typesafe wrapper for char** rather than void**. Unfortunately C won't implicitly cast this. */ static inline void free_many_charp(char **c, size_t n) { From d1278e1847392ef5c46acd0f1da689d5c986b3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 3 Mar 2026 09:55:09 +0100 Subject: [PATCH 3/6] basic/alloc-util: make realloc0 non-inline It's actually only used in one place in libsystemd and moving it even makes libsystemd smaller (in a non-optimized build): $ ls -l build/libsystemd.so.0.43.0* -rwxr-xr-x 1 zbyszek zbyszek 5763336 Mar 3 09:54 build/libsystemd.so.0.43.0-old -rwxr-xr-x 1 zbyszek zbyszek 5763216 Mar 3 09:54 build/libsystemd.so.0.43.0 Also, move the definitions in the .h file so that similar functions are grouped together and then move the definitions around in the .c file so that they are in the same order as in the header. --- src/basic/alloc-util.c | 56 ++++++++++++++++++++++---------- src/basic/alloc-util.h | 74 ++++++++++++++++-------------------------- 2 files changed, 66 insertions(+), 64 deletions(-) diff --git a/src/basic/alloc-util.c b/src/basic/alloc-util.c index 94102dd430..1eb98c1199 100644 --- a/src/basic/alloc-util.c +++ b/src/basic/alloc-util.c @@ -34,6 +34,44 @@ void* memdup_suffix0(const void *p, size_t l) { return memcpy_safe(ret, p, l); } +size_t malloc_sizeof_safe(void **xp) { + if (_unlikely_(!xp || !*xp)) + return 0; + + size_t sz = malloc_usable_size(*xp); + *xp = expand_to_usable(*xp, sz); + /* GCC doesn't see the _returns_nonnull_ when built with ubsan, so yet another hint to make it doubly + * clear that expand_to_usable won't return NULL. + * See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79265 */ + if (!*xp) + assert_not_reached(); + return sz; +} + +void* expand_to_usable(void *ptr, size_t newsize _unused_) { + return ptr; +} + +void* realloc0(void *p, size_t new_size) { + size_t old_size; + void *q; + + /* Like realloc(), but initializes anything appended to zero */ + + old_size = MALLOC_SIZEOF_SAFE(p); + + q = realloc(p, new_size); + if (!q) + return NULL; + + new_size = MALLOC_SIZEOF_SAFE(q); /* Update with actually allocated space */ + + if (new_size > old_size) + memset((uint8_t*) q + old_size, 0, new_size - old_size); + + return q; +} + void* greedy_realloc( void **p, size_t need, @@ -125,24 +163,6 @@ void* greedy_realloc_append( return q; } -void *expand_to_usable(void *ptr, size_t newsize _unused_) { - return ptr; -} - -size_t malloc_sizeof_safe(void **xp) { - if (_unlikely_(!xp || !*xp)) - return 0; - - size_t sz = malloc_usable_size(*xp); - *xp = expand_to_usable(*xp, sz); - /* GCC doesn't see the _returns_nonnull_ when built with ubsan, so yet another hint to make it doubly - * clear that expand_to_usable won't return NULL. - * See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79265 */ - if (!*xp) - assert_not_reached(); - return sz; -} - void free_many(void **p, size_t n) { assert(p || n == 0); diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index 7d5c1da61f..2c40c4771c 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -108,6 +108,34 @@ static inline void *memdup_suffix0_multiply(const void *p, size_t need, size_t s return memdup_suffix0(p, size * need); } +size_t malloc_sizeof_safe(void **xp); + +/* This returns the number of usable bytes in a malloc()ed region as per malloc_usable_size(), which may + * return a value larger than the size that was actually allocated. Access to that additional memory is + * discouraged because it violates the C standard; a compiler cannot see that this as valid. To help the + * compiler out, the MALLOC_SIZEOF_SAFE macro 'allocates' the usable size using a dummy allocator function + * expand_to_usable. There is a possibility of malloc_usable_size() returning different values during the + * lifetime of an object, which may cause problems, but the glibc allocator does not do that at the moment. */ +#define MALLOC_SIZEOF_SAFE(x) \ + malloc_sizeof_safe((void**) &__builtin_choose_expr(__builtin_constant_p(x), (void*) { NULL }, (x))) + +/* Inspired by ELEMENTSOF() but operates on malloc()'ed memory areas: typesafely returns the number of items + * that fit into the specified memory block */ +#define MALLOC_ELEMENTSOF(x) \ + (__builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(x), typeof(&*(x))), \ + MALLOC_SIZEOF_SAFE(x)/sizeof((x)[0]), \ + VOID_0)) + +/* Dummy allocator to tell the compiler that the new size of p is newsize. The implementation returns the + * pointer as is; the only reason for its existence is as a conduit for the _alloc_ attribute. This must not + * be inlined (hence a non-static function with _noinline_ because LTO otherwise tries to inline it) because + * gcc then loses the attributes on the function. + * See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503 */ +void *expand_to_usable(void *p, size_t newsize) _alloc_(2) _returns_nonnull_ _noinline_; + +void* realloc0(void *p, size_t new_size) _alloc_(2); + static inline size_t GREEDY_ALLOC_ROUND_UP(size_t l) { size_t m; @@ -179,32 +207,6 @@ void* greedy_realloc_append(void **p, size_t *n_p, const void *from, size_t n_fr # define msan_unpoison(r, s) #endif -/* Dummy allocator to tell the compiler that the new size of p is newsize. The implementation returns the - * pointer as is; the only reason for its existence is as a conduit for the _alloc_ attribute. This must not - * be inlined (hence a non-static function with _noinline_ because LTO otherwise tries to inline it) because - * gcc then loses the attributes on the function. - * See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503 */ -void *expand_to_usable(void *p, size_t newsize) _alloc_(2) _returns_nonnull_ _noinline_; - -size_t malloc_sizeof_safe(void **xp); - -/* This returns the number of usable bytes in a malloc()ed region as per malloc_usable_size(), which may - * return a value larger than the size that was actually allocated. Access to that additional memory is - * discouraged because it violates the C standard; a compiler cannot see that this as valid. To help the - * compiler out, the MALLOC_SIZEOF_SAFE macro 'allocates' the usable size using a dummy allocator function - * expand_to_usable. There is a possibility of malloc_usable_size() returning different values during the - * lifetime of an object, which may cause problems, but the glibc allocator does not do that at the moment. */ -#define MALLOC_SIZEOF_SAFE(x) \ - malloc_sizeof_safe((void**) &__builtin_choose_expr(__builtin_constant_p(x), (void*) { NULL }, (x))) - -/* Inspired by ELEMENTSOF() but operates on malloc()'ed memory areas: typesafely returns the number of items - * that fit into the specified memory block */ -#define MALLOC_ELEMENTSOF(x) \ - (__builtin_choose_expr( \ - __builtin_types_compatible_p(typeof(x), typeof(&*(x))), \ - MALLOC_SIZEOF_SAFE(x)/sizeof((x)[0]), \ - VOID_0)) - /* Free every element of the array. */ void free_many(void **p, size_t n); @@ -212,23 +214,3 @@ void free_many(void **p, size_t n); static inline void free_many_charp(char **c, size_t n) { free_many((void**) c, n); } - -_alloc_(2) static inline void *realloc0(void *p, size_t new_size) { - size_t old_size; - void *q; - - /* Like realloc(), but initializes anything appended to zero */ - - old_size = MALLOC_SIZEOF_SAFE(p); - - q = realloc(p, new_size); - if (!q) - return NULL; - - new_size = MALLOC_SIZEOF_SAFE(q); /* Update with actually allocated space */ - - if (new_size > old_size) - memset((uint8_t*) q + old_size, 0, new_size - old_size); - - return q; -} From 52abc9fe9655b21329eda7cf7a7d73fc3a7bdde7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 3 Mar 2026 10:00:29 +0100 Subject: [PATCH 4/6] basic/stdio-util: introduce asprintf_safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit asprintf is nice to use, but the _documented_ error return convention is unclear: > If memory allocation wasn't possible, or some other error occurs, > these functions will return -1, and the contents of strp are undefined. What exactly "undefined" means is up for debate: if it was really undefined, the caller wouldn't be able to meaningfully clean up, because they wouldn't know if strp is a valid pointer. So far we interpreted "undefined" — in some parts of the code base — as "either NULL or a valid pointer that needs to be freed", and — in other parts of the codebase — as "always NULL". I checked glibc and musl, and they both uncoditionally set the output pointer to NULL on failure. There is also no information _why_ asprintf failed. It could be an allocation error or format string error. But we just don't have this information. Let's add a wrapper that either returns a good string or a NULL pointer. Since there's just one failure result, we don't need a separate return value and an output argument and can simplify callers. --- docs/CODING_STYLE.md | 6 +++--- src/basic/meson.build | 1 + src/basic/stdio-util.c | 18 ++++++++++++++++++ src/basic/stdio-util.h | 2 ++ 4 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 src/basic/stdio-util.c diff --git a/docs/CODING_STYLE.md b/docs/CODING_STYLE.md index 767ab6734b..085c97df5c 100644 --- a/docs/CODING_STYLE.md +++ b/docs/CODING_STYLE.md @@ -754,9 +754,9 @@ SPDX-License-Identifier: LGPL-2.1-or-later section of the `alloca(3)` man page. - If you want to concatenate two or more strings, consider using `strjoina()` - or `strjoin()` rather than `asprintf()`, as the latter is a lot slower. This - matters particularly in inner loops (but note that `strjoina()` cannot be - used there). + or `strjoin()` rather than `asprintf()` or `asprintf_safe`, as the latter is + a lot slower. This matters particularly in inner loops (but note that + `strjoina()` cannot be used there). ## Runtime Behaviour diff --git a/src/basic/meson.build b/src/basic/meson.build index b8ffa80244..775dc1fa3d 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -99,6 +99,7 @@ basic_sources = files( 'sort-util.c', 'stat-util.c', 'static-destruct.c', + 'stdio-util.c', 'strbuf.c', 'string-table.c', 'string-util.c', diff --git a/src/basic/stdio-util.c b/src/basic/stdio-util.c new file mode 100644 index 0000000000..53267f08e2 --- /dev/null +++ b/src/basic/stdio-util.c @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "alloc-util.h" +#include "stdio-util.h" + +char* asprintf_safe(const char *restrict fmt, ...) { + _cleanup_free_ char *buf = NULL; + va_list ap; + int r; + + va_start(ap, fmt); + r = vasprintf(&buf, fmt, ap); + va_end(ap); + + if (r < 0) + return NULL; + return TAKE_PTR(buf); +} diff --git a/src/basic/stdio-util.h b/src/basic/stdio-util.h index f8ab0f0012..f8055b3853 100644 --- a/src/basic/stdio-util.h +++ b/src/basic/stdio-util.h @@ -21,6 +21,8 @@ static inline char* snprintf_ok(char *buf, size_t len, const char *format, ...) #define xsprintf(buf, fmt, ...) \ assert_message_se(snprintf_ok(buf, ELEMENTSOF(buf), fmt, ##__VA_ARGS__), "xsprintf: buffer too small") +char* asprintf_safe(const char *restrict fmt, ...) _printf_(1, 2); + #define VA_FORMAT_ADVANCE(format, ap) \ do { \ int _argtypes[128]; \ From 1dc6445bbb911b962ee6e8ef59d56f515791cae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 2 Mar 2026 15:35:41 +0100 Subject: [PATCH 5/6] shared/format-table: fix potential memleaks of d->formatted We don't always return d->formatted, even if it is available. And depending on the cell type, we'd either overwrite it directly or free first. Let's always free it upfront and then set unconditionally. (In this case, we don't need to spend effort on preserving the existing value. It's just a cache.) Setting the variable directly allows many temporary variables to be eliminated. Also use asprintf_safe() to simplify the allocation of the buffer. This is probably a tiny bit slower than the direct allocation, but table formatting shouldn't be a hot path. --- src/shared/format-table.c | 397 ++++++++------------------------------ 1 file changed, 84 insertions(+), 313 deletions(-) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 18cebf8628..f400602b34 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -1616,6 +1616,8 @@ static const char* table_data_format( (d->type != TABLE_STRV_WRAPPED || d->formatted_for_width == column_width)) return d->formatted; + d->formatted = mfree(d->formatted); + switch (d->type) { case TABLE_EMPTY: return table_ersatz_string(t); @@ -1649,18 +1651,13 @@ static const char* table_data_format( *q = 0; return d->formatted; - } else if (d->type == TABLE_FIELD) { - d->formatted = strjoin(s, ":"); - if (!d->formatted) - return NULL; - - return d->formatted; } - if (bn) { - d->formatted = TAKE_PTR(bn); - return d->formatted; - } + if (d->type == TABLE_FIELD) + return (d->formatted = strjoin(s, ":")); + + if (bn) + return (d->formatted = TAKE_PTR(bn)); return d->string; } @@ -1669,26 +1666,20 @@ static const char* table_data_format( if (strv_isempty(d->strv)) return table_ersatz_string(t); - d->formatted = strv_join(d->strv, "\n"); - if (!d->formatted) - return NULL; - break; + return (d->formatted = strv_join(d->strv, "\n")); - case TABLE_STRV_WRAPPED: { + case TABLE_STRV_WRAPPED: if (strv_isempty(d->strv)) return table_ersatz_string(t); - char *buf = format_strv_width(d->strv, column_width); - if (!buf) + d->formatted = format_strv_width(d->strv, column_width); + if (!d->formatted) return NULL; - free_and_replace(d->formatted, buf); d->formatted_for_width = column_width; if (have_soft) *have_soft = true; - - break; - } + return d->formatted; case TABLE_BOOLEAN: return yes_no(d->boolean); @@ -1702,12 +1693,12 @@ static const char* table_data_format( case TABLE_TIMESTAMP_RELATIVE_MONOTONIC: case TABLE_TIMESTAMP_LEFT: case TABLE_TIMESTAMP_DATE: { - _cleanup_free_ char *p = NULL; char *ret; - p = new(char, - IN_SET(d->type, TABLE_TIMESTAMP_RELATIVE, TABLE_TIMESTAMP_RELATIVE_MONOTONIC, TABLE_TIMESTAMP_LEFT) ? - FORMAT_TIMESTAMP_RELATIVE_MAX : FORMAT_TIMESTAMP_MAX); + _cleanup_free_ char *p = new( + char, + IN_SET(d->type, TABLE_TIMESTAMP_RELATIVE, TABLE_TIMESTAMP_RELATIVE_MONOTONIC, TABLE_TIMESTAMP_LEFT) ? + FORMAT_TIMESTAMP_RELATIVE_MAX : FORMAT_TIMESTAMP_MAX); if (!p) return NULL; @@ -1726,16 +1717,13 @@ static const char* table_data_format( if (!ret) return "-"; - d->formatted = TAKE_PTR(p); - break; + return (d->formatted = TAKE_PTR(p)); } case TABLE_TIMESPAN: case TABLE_TIMESPAN_MSEC: case TABLE_TIMESPAN_DAY: { - _cleanup_free_ char *p = NULL; - - p = new(char, FORMAT_TIMESPAN_MAX); + _cleanup_free_ char *p = new(char, FORMAT_TIMESPAN_MAX); if (!p) return NULL; @@ -1744,344 +1732,137 @@ static const char* table_data_format( d->type == TABLE_TIMESPAN_MSEC ? USEC_PER_MSEC : USEC_PER_DAY)) return "-"; - d->formatted = TAKE_PTR(p); - break; + return (d->formatted = TAKE_PTR(p)); } case TABLE_SIZE: { - _cleanup_free_ char *p = NULL; - - p = new(char, FORMAT_BYTES_MAX); + _cleanup_free_ char *p = new(char, FORMAT_BYTES_MAX); if (!p) return NULL; if (!format_bytes(p, FORMAT_BYTES_MAX, d->size)) return table_ersatz_string(t); - d->formatted = TAKE_PTR(p); - break; + return (d->formatted = TAKE_PTR(p)); } case TABLE_BPS: { - _cleanup_free_ char *p = NULL; - size_t n; - - p = new(char, FORMAT_BYTES_MAX+2); + _cleanup_free_ char *p = new(char, FORMAT_BYTES_MAX+2); if (!p) return NULL; if (!format_bytes_full(p, FORMAT_BYTES_MAX, d->size, FORMAT_BYTES_BELOW_POINT)) return table_ersatz_string(t); - n = strlen(p); + size_t n = strlen(p); strscpy(p + n, FORMAT_BYTES_MAX + 2 - n, "bps"); - d->formatted = TAKE_PTR(p); - break; + return (d->formatted = TAKE_PTR(p)); } - case TABLE_INT: { - _cleanup_free_ char *p = NULL; + case TABLE_INT: + return (d->formatted = asprintf_safe("%i", d->int_val)); - p = new(char, DECIMAL_STR_WIDTH(d->int_val) + 1); - if (!p) - return NULL; + case TABLE_INT8: + return (d->formatted = asprintf_safe("%" PRIi8, d->int8)); - sprintf(p, "%i", d->int_val); - d->formatted = TAKE_PTR(p); - break; - } + case TABLE_INT16: + return (d->formatted = asprintf_safe("%" PRIi16, d->int16)); - case TABLE_INT8: { - _cleanup_free_ char *p = NULL; + case TABLE_INT32: + return (d->formatted = asprintf_safe("%" PRIi32, d->int32)); - p = new(char, DECIMAL_STR_WIDTH(d->int8) + 1); - if (!p) - return NULL; + case TABLE_INT64: + return (d->formatted = asprintf_safe("%" PRIi64, d->int64)); - sprintf(p, "%" PRIi8, d->int8); - d->formatted = TAKE_PTR(p); - break; - } + case TABLE_UINT: + return (d->formatted = asprintf_safe("%u", d->uint_val)); - case TABLE_INT16: { - _cleanup_free_ char *p = NULL; + case TABLE_UINT8: + return (d->formatted = asprintf_safe("%" PRIu8, d->uint8)); - p = new(char, DECIMAL_STR_WIDTH(d->int16) + 1); - if (!p) - return NULL; + case TABLE_UINT16: + return (d->formatted = asprintf_safe("%" PRIu16, d->uint16)); - sprintf(p, "%" PRIi16, d->int16); - d->formatted = TAKE_PTR(p); - break; - } + case TABLE_UINT32: + return (d->formatted = asprintf_safe("%" PRIu32, d->uint32)); - case TABLE_INT32: { - _cleanup_free_ char *p = NULL; + case TABLE_UINT32_HEX: + return (d->formatted = asprintf_safe("%" PRIx32, d->uint32)); - p = new(char, DECIMAL_STR_WIDTH(d->int32) + 1); - if (!p) - return NULL; + case TABLE_UINT32_HEX_0x: + return (d->formatted = asprintf_safe("0x%" PRIx32, d->uint32)); - sprintf(p, "%" PRIi32, d->int32); - d->formatted = TAKE_PTR(p); - break; - } + case TABLE_UINT64: + return (d->formatted = asprintf_safe("%" PRIu64, d->uint64)); - case TABLE_INT64: { - _cleanup_free_ char *p = NULL; + case TABLE_UINT64_HEX: + return (d->formatted = asprintf_safe("%" PRIx64, d->uint64)); - p = new(char, DECIMAL_STR_WIDTH(d->int64) + 1); - if (!p) - return NULL; + case TABLE_UINT64_HEX_0x: + return (d->formatted = asprintf_safe("0x%" PRIx64, d->uint64)); - sprintf(p, "%" PRIi64, d->int64); - d->formatted = TAKE_PTR(p); - break; - } + case TABLE_PERCENT: + return (d->formatted = asprintf_safe("%i%%" , d->percent)); - case TABLE_UINT: { - _cleanup_free_ char *p = NULL; - - p = new(char, DECIMAL_STR_WIDTH(d->uint_val) + 1); - if (!p) - return NULL; - - sprintf(p, "%u", d->uint_val); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_UINT8: { - _cleanup_free_ char *p = NULL; - - p = new(char, DECIMAL_STR_WIDTH(d->uint8) + 1); - if (!p) - return NULL; - - sprintf(p, "%" PRIu8, d->uint8); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_UINT16: { - _cleanup_free_ char *p = NULL; - - p = new(char, DECIMAL_STR_WIDTH(d->uint16) + 1); - if (!p) - return NULL; - - sprintf(p, "%" PRIu16, d->uint16); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_UINT32: { - _cleanup_free_ char *p = NULL; - - p = new(char, DECIMAL_STR_WIDTH(d->uint32) + 1); - if (!p) - return NULL; - - sprintf(p, "%" PRIu32, d->uint32); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_UINT32_HEX: { - _cleanup_free_ char *p = NULL; - - p = new(char, 8 + 1); - if (!p) - return NULL; - - sprintf(p, "%" PRIx32, d->uint32); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_UINT32_HEX_0x: { - _cleanup_free_ char *p = NULL; - - p = new(char, 2 + 8 + 1); - if (!p) - return NULL; - - sprintf(p, "0x%" PRIx32, d->uint32); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_UINT64: { - _cleanup_free_ char *p = NULL; - - p = new(char, DECIMAL_STR_WIDTH(d->uint64) + 1); - if (!p) - return NULL; - - sprintf(p, "%" PRIu64, d->uint64); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_UINT64_HEX: { - _cleanup_free_ char *p = NULL; - - p = new(char, 16 + 1); - if (!p) - return NULL; - - sprintf(p, "%" PRIx64, d->uint64); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_UINT64_HEX_0x: { - _cleanup_free_ char *p = NULL; - - p = new(char, 2 + 16 + 1); - if (!p) - return NULL; - - sprintf(p, "0x%" PRIx64, d->uint64); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_PERCENT: { - _cleanup_free_ char *p = NULL; - - p = new(char, DECIMAL_STR_WIDTH(d->percent) + 2); - if (!p) - return NULL; - - sprintf(p, "%i%%" , d->percent); - d->formatted = TAKE_PTR(p); - break; - } - - case TABLE_IFINDEX: { - _cleanup_free_ char *p = NULL; - - if (format_ifname_full_alloc(d->ifindex, FORMAT_IFNAME_IFINDEX, &p) < 0) - return NULL; - - d->formatted = TAKE_PTR(p); - break; - } + case TABLE_IFINDEX: + (void) format_ifname_full_alloc(d->ifindex, FORMAT_IFNAME_IFINDEX, &d->formatted); + return d->formatted; case TABLE_IN_ADDR: - case TABLE_IN6_ADDR: { - _cleanup_free_ char *p = NULL; + case TABLE_IN6_ADDR: + (void) in_addr_to_string(d->type == TABLE_IN_ADDR ? AF_INET : AF_INET6, + &d->address, + &d->formatted); + return d->formatted; - if (in_addr_to_string(d->type == TABLE_IN_ADDR ? AF_INET : AF_INET6, - &d->address, &p) < 0) + case TABLE_ID128: + d->formatted = new(char, SD_ID128_STRING_MAX); + if (!d->formatted) return NULL; - d->formatted = TAKE_PTR(p); - break; - } + return sd_id128_to_string(d->id128, d->formatted); - case TABLE_ID128: { - char *p; - - p = new(char, SD_ID128_STRING_MAX); - if (!p) + case TABLE_UUID: + d->formatted = new(char, SD_ID128_UUID_STRING_MAX); + if (!d->formatted) return NULL; - d->formatted = sd_id128_to_string(d->id128, p); - break; - } - - case TABLE_UUID: { - char *p; - - p = new(char, SD_ID128_UUID_STRING_MAX); - if (!p) - return NULL; - - d->formatted = sd_id128_to_uuid_string(d->id128, p); - break; - } - - case TABLE_UID: { - char *p; + return sd_id128_to_uuid_string(d->id128, d->formatted); + case TABLE_UID: if (!uid_is_valid(d->uid)) return table_ersatz_string(t); - p = new(char, DECIMAL_STR_WIDTH(d->uid) + 1); - if (!p) - return NULL; - sprintf(p, UID_FMT, d->uid); - - d->formatted = p; - break; - } - - case TABLE_GID: { - char *p; + return (d->formatted = asprintf_safe(UID_FMT, d->uid)); + case TABLE_GID: if (!gid_is_valid(d->gid)) return table_ersatz_string(t); - p = new(char, DECIMAL_STR_WIDTH(d->gid) + 1); - if (!p) - return NULL; - sprintf(p, GID_FMT, d->gid); - - d->formatted = p; - break; - } - - case TABLE_PID: { - char *p; + return (d->formatted = asprintf_safe(GID_FMT, d->gid)); + case TABLE_PID: if (!pid_is_valid(d->pid)) return table_ersatz_string(t); - p = new(char, DECIMAL_STR_WIDTH(d->pid) + 1); - if (!p) - return NULL; - sprintf(p, PID_FMT, d->pid); - - d->formatted = p; - break; - } + return (d->formatted = asprintf_safe(PID_FMT, d->pid)); case TABLE_SIGNAL: { - const char *suffix; - char *p; - - suffix = signal_to_string(d->int_val); + const char *suffix = signal_to_string(d->int_val); if (!suffix) return table_ersatz_string(t); - p = strjoin("SIG", suffix); - if (!p) - return NULL; - - d->formatted = p; - break; + return (d->formatted = strjoin("SIG", suffix)); } - case TABLE_MODE: { - char *p; - + case TABLE_MODE: if (d->mode == MODE_INVALID) return table_ersatz_string(t); - p = new(char, 4 + 1); - if (!p) - return NULL; - - sprintf(p, "%04o", d->mode & 07777); - d->formatted = p; - break; - } + return (d->formatted = asprintf_safe("%04o", d->mode & 07777)); case TABLE_MODE_INODE_TYPE: - if (d->mode == MODE_INVALID) return table_ersatz_string(t); @@ -2091,28 +1872,18 @@ static const char* table_data_format( if (devnum_is_zero(d->devnum)) return table_ersatz_string(t); - if (asprintf(&d->formatted, DEVNUM_FORMAT_STR, DEVNUM_FORMAT_VAL(d->devnum)) < 0) - return NULL; + return (d->formatted = asprintf_safe(DEVNUM_FORMAT_STR, DEVNUM_FORMAT_VAL(d->devnum))); - break; - - case TABLE_JSON: { + case TABLE_JSON: if (!d->json) return table_ersatz_string(t); - char *p; - if (sd_json_variant_format(d->json, /* flags= */ 0, &p) < 0) - return NULL; - - d->formatted = p; - break; - } + (void) sd_json_variant_format(d->json, /* flags= */ 0, &d->formatted); + return d->formatted; default: assert_not_reached(); } - - return d->formatted; } static const char* table_data_format_strip_ansi( From dc0b4388365ead78584db4e027f6e02904c3631f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 3 Mar 2026 10:51:11 +0100 Subject: [PATCH 6/6] various: use asprintf_safe in more places --- src/basic/in-addr-util.c | 14 +++--- src/basic/rlimit-util.c | 17 ++++--- src/basic/string-table.c | 9 ++-- src/bless-boot/bless-boot.c | 17 ++++--- src/core/execute.c | 12 ++--- src/core/socket.c | 96 +++++++++++++++++-------------------- 6 files changed, 75 insertions(+), 90 deletions(-) diff --git a/src/basic/in-addr-util.c b/src/basic/in-addr-util.c index a47bdf7149..957ce2a466 100644 --- a/src/basic/in-addr-util.c +++ b/src/basic/in-addr-util.c @@ -559,20 +559,18 @@ int in_addr_port_ifindex_name_to_string(int family, const union in_addr_union *u if (port > 0) { if (family == AF_INET6) { if (ifindex > 0) - r = asprintf(&x, "[%s]:%"PRIu16"%%%i%s%s", ip_str, port, ifindex, separator, server_name); + x = asprintf_safe("[%s]:%"PRIu16"%%%i%s%s", ip_str, port, ifindex, separator, server_name); else - r = asprintf(&x, "[%s]:%"PRIu16"%s%s", ip_str, port, separator, server_name); + x = asprintf_safe("[%s]:%"PRIu16"%s%s", ip_str, port, separator, server_name); } else - r = asprintf(&x, "%s:%"PRIu16"%s%s", ip_str, port, separator, server_name); + x = asprintf_safe("%s:%"PRIu16"%s%s", ip_str, port, separator, server_name); } else { if (ifindex > 0) - r = asprintf(&x, "%s%%%i%s%s", ip_str, ifindex, separator, server_name); - else { + x = asprintf_safe("%s%%%i%s%s", ip_str, ifindex, separator, server_name); + else x = strjoin(ip_str, separator, server_name); - r = x ? 0 : -ENOMEM; - } } - if (r < 0) + if (!x) return -ENOMEM; *ret = TAKE_PTR(x); diff --git a/src/basic/rlimit-util.c b/src/basic/rlimit-util.c index 331f2d1a5b..ba1af81bb4 100644 --- a/src/basic/rlimit-util.c +++ b/src/basic/rlimit-util.c @@ -289,26 +289,25 @@ int rlimit_parse(int resource, const char *val, struct rlimit *ret) { } int rlimit_format(const struct rlimit *rl, char **ret) { - _cleanup_free_ char *s = NULL; - int r; + char *s; assert(rl); assert(ret); if (rl->rlim_cur >= RLIM_INFINITY && rl->rlim_max >= RLIM_INFINITY) - r = free_and_strdup(&s, "infinity"); + s = strdup("infinity"); else if (rl->rlim_cur >= RLIM_INFINITY) - r = asprintf(&s, "infinity:" RLIM_FMT, rl->rlim_max); + s = asprintf_safe("infinity:" RLIM_FMT, rl->rlim_max); else if (rl->rlim_max >= RLIM_INFINITY) - r = asprintf(&s, RLIM_FMT ":infinity", rl->rlim_cur); + s = asprintf_safe(RLIM_FMT ":infinity", rl->rlim_cur); else if (rl->rlim_cur == rl->rlim_max) - r = asprintf(&s, RLIM_FMT, rl->rlim_cur); + s = asprintf_safe(RLIM_FMT, rl->rlim_cur); else - r = asprintf(&s, RLIM_FMT ":" RLIM_FMT, rl->rlim_cur, rl->rlim_max); - if (r < 0) + s = asprintf_safe(RLIM_FMT ":" RLIM_FMT, rl->rlim_cur, rl->rlim_max); + if (!s) return -ENOMEM; - *ret = TAKE_PTR(s); + *ret = s; return 0; } diff --git a/src/basic/string-table.c b/src/basic/string-table.c index 069cb40ec1..461f94f9ee 100644 --- a/src/basic/string-table.c +++ b/src/basic/string-table.c @@ -3,6 +3,7 @@ #include #include "parse-util.h" +#include "stdio-util.h" #include "string-table.h" #include "string-util.h" @@ -43,11 +44,11 @@ int string_table_lookup_to_string_fallback(const char * const *table, size_t len if (i < 0 || i > (ssize_t) max) return -ERANGE; - if (i < (ssize_t) len && table[i]) { + if (i < (ssize_t) len && table[i]) s = strdup(table[i]); - if (!s) - return -ENOMEM; - } else if (asprintf(&s, "%zd", i) < 0) + else + s = asprintf_safe("%zd", i); + if (!s) return -ENOMEM; *ret = s; diff --git a/src/bless-boot/bless-boot.c b/src/bless-boot/bless-boot.c index 9c717ace9d..ac989630af 100644 --- a/src/bless-boot/bless-boot.c +++ b/src/bless-boot/bless-boot.c @@ -15,6 +15,7 @@ #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" +#include "stdio-util.h" #include "string-util.h" #include "strv.h" #include "sync-util.h" @@ -299,7 +300,7 @@ static int make_good(const char *prefix, const char *suffix, char **ret) { } static int make_bad(const char *prefix, uint64_t done, const char *suffix, char **ret) { - _cleanup_free_ char *bad = NULL; + char *bad; assert(prefix); assert(suffix); @@ -308,16 +309,14 @@ static int make_bad(const char *prefix, uint64_t done, const char *suffix, char /* Generate the path we'd use on bad boots. Let's simply set the 'left' counter to zero, and keep the 'done' * counter. The information might be interesting to boot loaders, after all. */ - if (done == 0) { + if (done == 0) bad = strjoin(prefix, "+0", suffix); - if (!bad) - return -ENOMEM; - } else { - if (asprintf(&bad, "%s+0-%" PRIu64 "%s", prefix, done, suffix) < 0) - return -ENOMEM; - } + else + bad = asprintf_safe("%s+0-%" PRIu64 "%s", prefix, done, suffix); + if (!bad) + return -ENOMEM; - *ret = TAKE_PTR(bad); + *ret = bad; return 0; } diff --git a/src/core/execute.c b/src/core/execute.c index 80c22623be..56fd650a48 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1930,14 +1930,12 @@ char** exec_context_get_syscall_filter(const ExecContext *c) { if (num >= 0) { e = seccomp_errno_or_action_to_string(num); - if (e) { + if (e) s = strjoin(name, ":", e); - if (!s) - return NULL; - } else { - if (asprintf(&s, "%s:%d", name, num) < 0) - return NULL; - } + else + s = asprintf_safe("%s:%d", name, num); + if (!s) + return NULL; } else s = TAKE_PTR(name); diff --git a/src/core/socket.c b/src/core/socket.c index c18f28aad6..43f61e456d 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -882,16 +882,14 @@ static int instance_from_socket( a = be32toh(local.in.sin_addr.s_addr), b = be32toh(remote.in.sin_addr.s_addr); - if (asprintf(&s, - "%u-%" PRIu64 "-%u.%u.%u.%u:%u-%u.%u.%u.%u:%u", - nr, - cookie, - a >> 24, (a >> 16) & 0xFF, (a >> 8) & 0xFF, a & 0xFF, - be16toh(local.in.sin_port), - b >> 24, (b >> 16) & 0xFF, (b >> 8) & 0xFF, b & 0xFF, - be16toh(remote.in.sin_port)) < 0) - return -ENOMEM; - + s = asprintf_safe( + "%u-%" PRIu64 "-%u.%u.%u.%u:%u-%u.%u.%u.%u:%u", + nr, + cookie, + a >> 24, (a >> 16) & 0xFF, (a >> 8) & 0xFF, a & 0xFF, + be16toh(local.in.sin_port), + b >> 24, (b >> 16) & 0xFF, (b >> 8) & 0xFF, b & 0xFF, + be16toh(remote.in.sin_port)); break; } @@ -906,27 +904,23 @@ static int instance_from_socket( *a = local.in6.sin6_addr.s6_addr+12, *b = remote.in6.sin6_addr.s6_addr+12; - if (asprintf(&s, - "%u-%" PRIu64 "-%u.%u.%u.%u:%u-%u.%u.%u.%u:%u", - nr, - cookie, - a[0], a[1], a[2], a[3], - be16toh(local.in6.sin6_port), - b[0], b[1], b[2], b[3], - be16toh(remote.in6.sin6_port)) < 0) - return -ENOMEM; - } else { - if (asprintf(&s, - "%u-%" PRIu64 "-%s:%u-%s:%u", - nr, - cookie, - IN6_ADDR_TO_STRING(&local.in6.sin6_addr), - be16toh(local.in6.sin6_port), - IN6_ADDR_TO_STRING(&remote.in6.sin6_addr), - be16toh(remote.in6.sin6_port)) < 0) - return -ENOMEM; - } - + s = asprintf_safe( + "%u-%" PRIu64 "-%u.%u.%u.%u:%u-%u.%u.%u.%u:%u", + nr, + cookie, + a[0], a[1], a[2], a[3], + be16toh(local.in6.sin6_port), + b[0], b[1], b[2], b[3], + be16toh(remote.in6.sin6_port)); + } else + s = asprintf_safe( + "%u-%" PRIu64 "-%s:%u-%s:%u", + nr, + cookie, + IN6_ADDR_TO_STRING(&local.in6.sin6_addr), + be16toh(local.in6.sin6_port), + IN6_ADDR_TO_STRING(&remote.in6.sin6_addr), + be16toh(remote.in6.sin6_port)); break; } @@ -939,42 +933,38 @@ static int instance_from_socket( uint64_t pidfd_id; if (pidfd >= 0 && pidfd_get_inode_id(pidfd, &pidfd_id) >= 0) - r = asprintf(&s, "%u-%" PRIu64 "-" PID_FMT "_%" PRIu64 "-" UID_FMT, - nr, cookie, ucred.pid, pidfd_id, ucred.uid); + s = asprintf_safe( + "%u-%" PRIu64 "-" PID_FMT "_%" PRIu64 "-" UID_FMT, + nr, cookie, ucred.pid, pidfd_id, ucred.uid); else - r = asprintf(&s, "%u-%" PRIu64 "-" PID_FMT "-" UID_FMT, - nr, cookie, ucred.pid, ucred.uid); - if (r < 0) - return -ENOMEM; - } else if (r == -ENODATA) { + s = asprintf_safe( + "%u-%" PRIu64 "-" PID_FMT "-" UID_FMT, + nr, cookie, ucred.pid, ucred.uid); + } else if (r == -ENODATA) /* This handles the case where somebody is connecting from another pid/uid namespace * (e.g. from outside of our container). */ - if (asprintf(&s, - "%u-%" PRIu64 "-unknown", - nr, - cookie) < 0) - return -ENOMEM; - } else + s = asprintf_safe("%u-%" PRIu64 "-unknown", nr, cookie); + else return r; - break; } case AF_VSOCK: - if (asprintf(&s, - "%u-%" PRIu64 "-%u:%u-%u:%u", - nr, - cookie, - local.vm.svm_cid, local.vm.svm_port, - remote.vm.svm_cid, remote.vm.svm_port) < 0) - return -ENOMEM; - + s = asprintf_safe( + "%u-%" PRIu64 "-%u:%u-%u:%u", + nr, + cookie, + local.vm.svm_cid, local.vm.svm_port, + remote.vm.svm_cid, remote.vm.svm_port); break; default: assert_not_reached(); } + if (!s) + return -ENOMEM; + *ret = s; return 0; }