From fdd84270df0062fad68783eea8b51a6ed87b67cd Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 16 Jan 2024 22:42:39 +0100 Subject: [PATCH 1/3] Reorder arguments for calloc()-like functions, part #2 To appease gcc-14's -Wcalloc-transposed-args check. Follow-up for 2a9ab0974bb290bc66dc84d909c33d23361b0752. --- src/basic/alloc-util.h | 4 ++-- src/boot/efi/util.h | 4 ++-- src/nspawn/nspawn-bind-user.c | 2 +- src/test/test-alloc-util.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index 05a6f211f7..136d2b3e68 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -47,7 +47,7 @@ typedef void* (*mfree_func_t)(void *p); #define newdup(t, p, n) ((t*) memdup_multiply(p, (n), sizeof(t))) -#define newdup_suffix0(t, p, n) ((t*) memdup_suffix0_multiply(p, sizeof(t), (n))) +#define newdup_suffix0(t, p, n) ((t*) memdup_suffix0_multiply(p, (n), sizeof(t))) #define malloc0(n) (calloc(1, (n) ?: 1)) @@ -137,7 +137,7 @@ _alloc_(2, 3) static inline void *memdup_multiply(const void *p, size_t need, si /* Note that we can't decorate this function with _alloc_() since the returned memory area is one byte larger * than the product of its parameters. */ -static inline void *memdup_suffix0_multiply(const void *p, size_t size, size_t need) { +static inline void *memdup_suffix0_multiply(const void *p, size_t need, size_t size) { if (size_multiply_overflow(size, need)) return NULL; diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index aef831d132..6e15a8b85d 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -36,7 +36,7 @@ static inline void *xmalloc(size_t size) { } _malloc_ _alloc_(1, 2) _returns_nonnull_ _warn_unused_result_ -static inline void *xmalloc_multiply(size_t size, size_t n) { +static inline void *xmalloc_multiply(size_t n, size_t size) { assert_se(!__builtin_mul_overflow(size, n, &size)); return xmalloc(size); } @@ -57,7 +57,7 @@ static inline void* xmemdup(const void *p, size_t l) { return memcpy(xmalloc(l), p, l); } -#define xnew(type, n) ((type *) xmalloc_multiply(sizeof(type), (n))) +#define xnew(type, n) ((type *) xmalloc_multiply((n), sizeof(type))) typedef struct { EFI_PHYSICAL_ADDRESS addr; diff --git a/src/nspawn/nspawn-bind-user.c b/src/nspawn/nspawn-bind-user.c index c7e1a9253c..018e7a37e2 100644 --- a/src/nspawn/nspawn-bind-user.c +++ b/src/nspawn/nspawn-bind-user.c @@ -286,7 +286,7 @@ int bind_user_prepare( if (!sd) return log_oom(); - cm = reallocarray(*custom_mounts, sizeof(CustomMount), *n_custom_mounts + 1); + cm = reallocarray(*custom_mounts, *n_custom_mounts + 1, sizeof(CustomMount)); if (!cm) return log_oom(); diff --git a/src/test/test-alloc-util.c b/src/test/test-alloc-util.c index 57cb886c41..24cb5f73eb 100644 --- a/src/test/test-alloc-util.c +++ b/src/test/test-alloc-util.c @@ -100,7 +100,7 @@ TEST(memdup_multiply_and_greedy_realloc) { size_t i; int *p; - dup = memdup_suffix0_multiply(org, sizeof(int), 3); + dup = memdup_suffix0_multiply(org, 3, sizeof(int)); assert_se(dup); assert_se(dup[0] == 1); assert_se(dup[1] == 2); @@ -108,7 +108,7 @@ TEST(memdup_multiply_and_greedy_realloc) { assert_se(((uint8_t*) dup)[sizeof(int) * 3] == 0); free(dup); - dup = memdup_multiply(org, sizeof(int), 3); + dup = memdup_multiply(org, 3, sizeof(int)); assert_se(dup); assert_se(dup[0] == 1); assert_se(dup[1] == 2); From b59bce308df746e1793b134db7fec4c298ed1f61 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 16 Jan 2024 22:25:04 +0100 Subject: [PATCH 2/3] meson: disable -Wnonnull-compare MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This gets enabled by default in gcc-14 and complains everywhere where we use assert() on an expression that is always true (i.e. using `int x[static 2]` in function declaration, etc.): [153/2414] Compiling C object src/basic/libbasic.a.p/fs-util.c.o In file included from ../src/basic/macro.h:13, from ../src/basic/alloc-util.h:10, from ../src/basic/fs-util.c:11: ../src/basic/fd-util.h: In function ‘format_proc_fd_path’: ../src/fundamental/macro-fundamental.h:74:41: warning: ‘nonnull’ argument ‘buf’ compared to NULL [-Wnonnull-compare] 74 | #define _unlikely_(x) (__builtin_expect(!!(x), 0)) | ^~~~~ ../src/basic/macro.h:150:21: note: in expansion of macro ‘_unlikely_’ 150 | if (_unlikely_(!(expr))) \ | ^~~~~~~~~~ ../src/basic/macro.h:167:22: note: in expansion of macro ‘assert_message_se’ 167 | #define assert(expr) assert_message_se(expr, #expr) | ^~~~~~~~~~~~~~~~~ ../src/basic/fd-util.h:129:9: note: in expansion of macro ‘assert’ 129 | assert(buf); | ^~~~~~ Disabling this selectively only for asserts is a bit painful, since the option is not available in all compilers, and it'd need to be handled in the EFI stuff as well. --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index 53a3d966ff..02a0e9b8f8 100644 --- a/meson.build +++ b/meson.build @@ -341,6 +341,7 @@ endif basic_disabled_warnings = [ '-Wno-missing-field-initializers', '-Wno-unused-parameter', + '-Wno-nonnull-compare', ] possible_common_cc_flags = [ From dc571cccd75db7be49b2aada64baf92e3a498c39 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 17 Jan 2024 13:11:14 +0100 Subject: [PATCH 3/3] macro: terminate the temporary VA_ARGS_FOREACH() array with a sentinel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So gcc-14 doesn't complain we're out of bounds on the last iteration: [2092/2414] Compiling C object test-macro.p/src_test_test-macro.c.o In file included from ../src/basic/list.h:209, from ../src/basic/log.h:10, from ../src/test/test-macro.c:5: ../src/test/test-macro.c: In function ‘test_FOREACH_VA_ARGS’: ../src/basic/macro.h:395:90: warning: array subscript 1 is outside array bounds of ‘uint8_t[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds=] 395 | ((long)(_current_ - _entries_) < (long)ELEMENTSOF(_entries_)) && ({ entry = *_current_; true; }); \ ../src/basic/macro.h:392:9: note: in expansion of macro ‘_VA_ARGS_FOREACH’ 392 | _VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~ ../src/test/test-macro.c:322:9: note: in expansion of macro ‘VA_ARGS_FOREACH’ 322 | VA_ARGS_FOREACH(u8, 0) { | ^~~~~~~~~~~~~~~ ../src/fundamental/macro-fundamental.h:163:37: note: at offset 1 into object ‘__unique_prefix__entries_181’ of size 1 163 | #define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq)) | ^~~~~~~~~~~~~~~~ ../src/basic/macro.h:394:28: note: in definition of macro ‘_VA_ARGS_FOREACH’ 394 | for (typeof(entry) _entries_[] = { __VA_ARGS__ }, *_current_ = _entries_; \ | ^~~~~~~~~ ../src/fundamental/macro-fundamental.h:109:27: note: in expansion of macro ‘XCONCATENATE’ 109 | #define CONCATENATE(x, y) XCONCATENATE(x, y) | ^~~~~~~~~~~~ ../src/fundamental/macro-fundamental.h:163:25: note: in expansion of macro ‘CONCATENATE’ 163 | #define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq)) | ^~~~~~~~~~~ ../src/basic/macro.h:392:33: note: in expansion of macro ‘UNIQ_T’ 392 | _VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), ##__VA_ARGS__) | ^~~~~~ ../src/test/test-macro.c:322:9: note: in expansion of macro ‘VA_ARGS_FOREACH’ 322 | VA_ARGS_FOREACH(u8, 0) { | ^~~~~~~~~~~~~~~ --- src/basic/macro.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/basic/macro.h b/src/basic/macro.h index d3eb980abd..d63aa816cc 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -383,10 +383,10 @@ assert_cc(sizeof(dummy_t) == 0); /* Iterate through each variadic arg. All must be the same type as 'entry' or must be implicitly * convertible. The iteration variable 'entry' must already be defined. */ #define VA_ARGS_FOREACH(entry, ...) \ - _VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), ##__VA_ARGS__) -#define _VA_ARGS_FOREACH(entry, _entries_, _current_, ...) \ - for (typeof(entry) _entries_[] = { __VA_ARGS__ }, *_current_ = _entries_; \ - ((long)(_current_ - _entries_) < (long)ELEMENTSOF(_entries_)) && ({ entry = *_current_; true; }); \ + _VA_ARGS_FOREACH(entry, UNIQ_T(_entries_, UNIQ), UNIQ_T(_current_, UNIQ), UNIQ_T(_va_sentinel_, UNIQ), ##__VA_ARGS__) +#define _VA_ARGS_FOREACH(entry, _entries_, _current_, _va_sentinel_, ...) \ + for (typeof(entry) _va_sentinel_[1] = {}, _entries_[] = { __VA_ARGS__ __VA_OPT__(,) _va_sentinel_[0] }, *_current_ = _entries_; \ + ((long)(_current_ - _entries_) < (long)(ELEMENTSOF(_entries_) - 1)) && ({ entry = *_current_; true; }); \ _current_++) #include "log.h"