From 3facdc7da8ad424a38ce9c673fbb94a41e070a7d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 7 May 2023 18:34:35 +0900 Subject: [PATCH 1/3] memory-util: make ArrayCleanup passed to array_cleanup() const Should not change any behavior, preparation for later commits. --- src/basic/memory-util.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/basic/memory-util.h b/src/basic/memory-util.h index d03d52cd43..d26a0918e1 100644 --- a/src/basic/memory-util.h +++ b/src/basic/memory-util.h @@ -113,13 +113,13 @@ static inline void erase_char(char *p) { } /* An automatic _cleanup_-like logic for destroy arrays (i.e. pointers + size) when leaving scope */ -struct ArrayCleanup { +typedef struct ArrayCleanup { void **parray; size_t *pn; free_array_func_t pfunc; -}; +} ArrayCleanup; -static inline void array_cleanup(struct ArrayCleanup *c) { +static inline void array_cleanup(const ArrayCleanup *c) { assert(c); assert(!c->parray == !c->pn); @@ -137,7 +137,7 @@ static inline void array_cleanup(struct ArrayCleanup *c) { } #define CLEANUP_ARRAY(array, n, func) \ - _cleanup_(array_cleanup) _unused_ struct ArrayCleanup CONCATENATE(_cleanup_array_, UNIQ) = { \ + _cleanup_(array_cleanup) _unused_ const ArrayCleanup CONCATENATE(_cleanup_array_, UNIQ) = { \ .parray = (void**) &(array), \ .pn = &(n), \ .pfunc = (free_array_func_t) ({ \ From 555ead898539183a435e18c6e1e4d5fb89499231 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 7 May 2023 18:37:13 +0900 Subject: [PATCH 2/3] static-destruct: several cleanups No functional changes, preparation for later commits. --- src/basic/static-destruct.h | 42 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/basic/static-destruct.h b/src/basic/static-destruct.h index 97baac7abb..4bc82889be 100644 --- a/src/basic/static-destruct.h +++ b/src/basic/static-destruct.h @@ -10,6 +10,21 @@ * feel a bit like the gcc cleanup attribute, but for static variables. Note that this does not work for static * variables declared in .so's, as the list is private to the same linking unit. But maybe that's a good thing. */ +#define _common_static_destruct_attrs_ \ + /* Older compilers don't know "retain" attribute. */ \ + _Pragma("GCC diagnostic ignored \"-Wattributes\"") \ + /* The actual destructor structure we place in a special section to find it. */ \ + _section_("SYSTEMD_STATIC_DESTRUCT") \ + /* Use pointer alignment, since that is apparently what gcc does for static variables. */ \ + _alignptr_ \ + /* Make sure this is not dropped from the image despite not being explicitly referenced. */ \ + _used_ \ + /* Prevent garbage collection by the linker. */ \ + _retain_ \ + /* Make sure that AddressSanitizer doesn't pad this variable: we want everything in this section + * packed next to each other so that we can enumerate it. */ \ + _variable_no_sanitize_address_ + typedef struct StaticDestructor { void *data; free_func_t destroy; @@ -24,19 +39,7 @@ typedef struct StaticDestructor { typeof(variable) *q = p; \ func(q); \ } \ - /* Older compilers don't know "retain" attribute. */ \ - _Pragma("GCC diagnostic ignored \"-Wattributes\"") \ - /* The actual destructor structure we place in a special section to find it. */ \ - _section_("SYSTEMD_STATIC_DESTRUCT") \ - /* Use pointer alignment, since that is apparently what gcc does for static variables. */ \ - _alignptr_ \ - /* Make sure this is not dropped from the image despite not being explicitly referenced. */ \ - _used_ \ - /* Prevent garbage collection by the linker. */ \ - _retain_ \ - /* Make sure that AddressSanitizer doesn't pad this variable: we want everything in this section - * packed next to each other so that we can enumerate it. */ \ - _variable_no_sanitize_address_ \ + _common_static_destruct_attrs_ \ static const StaticDestructor UNIQ_T(static_destructor_entry, uq) = { \ .data = &(variable), \ .destroy = UNIQ_T(static_destructor_wrapper, uq), \ @@ -44,20 +47,17 @@ typedef struct StaticDestructor { /* Beginning and end of our section listing the destructors. We define these as weak as we want this to work * even if no destructors are defined and the section is missing. */ -extern const struct StaticDestructor _weak_ __start_SYSTEMD_STATIC_DESTRUCT[]; -extern const struct StaticDestructor _weak_ __stop_SYSTEMD_STATIC_DESTRUCT[]; +extern const StaticDestructor _weak_ __start_SYSTEMD_STATIC_DESTRUCT[]; +extern const StaticDestructor _weak_ __stop_SYSTEMD_STATIC_DESTRUCT[]; /* The function to destroy everything. (Note that this must be static inline, as it's key that it remains in * the same linking unit as the variables we want to destroy.) */ static inline void static_destruct(void) { - const StaticDestructor *d; - if (!__start_SYSTEMD_STATIC_DESTRUCT) return; - d = ALIGN_PTR(__start_SYSTEMD_STATIC_DESTRUCT); - while (d < __stop_SYSTEMD_STATIC_DESTRUCT) { + for (const StaticDestructor *d = ALIGN_PTR(__start_SYSTEMD_STATIC_DESTRUCT); + d < __stop_SYSTEMD_STATIC_DESTRUCT; + d = ALIGN_PTR(d + 1)) d->destroy(d->data); - d = ALIGN_PTR(d + 1); - } } From 9695b0c01bf3d4b260432fb6754c7fbe9173c7db Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 9 May 2023 06:44:27 +0900 Subject: [PATCH 3/3] static-destruct: introduce STATIC_ARRAY_DESTRUCTOR_REGISTER() --- src/basic/static-destruct.h | 50 ++++++++++++++++++++++++++++++--- src/test/test-static-destruct.c | 45 +++++++++++++++++++++++++---- 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/basic/static-destruct.h b/src/basic/static-destruct.h index 4bc82889be..2ffc6516f8 100644 --- a/src/basic/static-destruct.h +++ b/src/basic/static-destruct.h @@ -4,6 +4,7 @@ #include "alloc-util.h" #include "macro.h" +#include "memory-util.h" /* A framework for registering static variables that shall be freed on shutdown of a process. It's a bit like gcc's * destructor attribute, but allows us to precisely schedule when we want to free the variables. This is supposed to @@ -25,9 +26,24 @@ * packed next to each other so that we can enumerate it. */ \ _variable_no_sanitize_address_ -typedef struct StaticDestructor { +typedef enum StaticDestructorType { + STATIC_DESTRUCTOR_SIMPLE, + STATIC_DESTRUCTOR_ARRAY, + _STATIC_DESTRUCTOR_TYPE_MAX, + _STATIC_DESTRUCTOR_INVALID = -EINVAL, +} StaticDestructorType; + +typedef struct SimpleCleanup { void *data; free_func_t destroy; +} SimpleCleanup; + +typedef struct StaticDestructor { + StaticDestructorType type; + union { + SimpleCleanup simple; + ArrayCleanup array; + }; } StaticDestructor; #define STATIC_DESTRUCTOR_REGISTER(variable, func) \ @@ -41,10 +57,25 @@ typedef struct StaticDestructor { } \ _common_static_destruct_attrs_ \ static const StaticDestructor UNIQ_T(static_destructor_entry, uq) = { \ - .data = &(variable), \ - .destroy = UNIQ_T(static_destructor_wrapper, uq), \ + .type = STATIC_DESTRUCTOR_SIMPLE, \ + .simple.data = &(variable), \ + .simple.destroy = UNIQ_T(static_destructor_wrapper, uq), \ } +#define STATIC_ARRAY_DESTRUCTOR_REGISTER(a, n, func) \ + _STATIC_ARRAY_DESTRUCTOR_REGISTER(UNIQ, a, n, func) + +#define _STATIC_ARRAY_DESTRUCTOR_REGISTER(uq, a, n, func) \ + /* Type-safety check */ \ + _unused_ static void (* UNIQ_T(static_destructor_wrapper, uq))(typeof(a[0]) *x, size_t y) = (func); \ + _common_static_destruct_attrs_ \ + static const StaticDestructor UNIQ_T(static_destructor_entry, uq) = { \ + .type = STATIC_DESTRUCTOR_ARRAY, \ + .array.parray = (void**) &(a), \ + .array.pn = &(n), \ + .array.pfunc = (free_array_func_t) (func), \ + }; + /* Beginning and end of our section listing the destructors. We define these as weak as we want this to work * even if no destructors are defined and the section is missing. */ extern const StaticDestructor _weak_ __start_SYSTEMD_STATIC_DESTRUCT[]; @@ -59,5 +90,16 @@ static inline void static_destruct(void) { for (const StaticDestructor *d = ALIGN_PTR(__start_SYSTEMD_STATIC_DESTRUCT); d < __stop_SYSTEMD_STATIC_DESTRUCT; d = ALIGN_PTR(d + 1)) - d->destroy(d->data); + switch (d->type) { + case STATIC_DESTRUCTOR_SIMPLE: + d->simple.destroy(d->simple.data); + break; + + case STATIC_DESTRUCTOR_ARRAY: + array_cleanup(&d->array); + break; + + default: + assert_not_reached(); + } } diff --git a/src/test/test-static-destruct.c b/src/test/test-static-destruct.c index cb518ea362..ef8648f588 100644 --- a/src/test/test-static-destruct.c +++ b/src/test/test-static-destruct.c @@ -2,17 +2,38 @@ #include "alloc-util.h" #include "static-destruct.h" +#include "strv.h" #include "tests.h" static int foo = 0; static int bar = 0; static int baz = 0; -static char* memory = NULL; +static char *memory = NULL; +static char **strings = NULL; +static size_t n_strings = 0; +static int *integers = NULL; +static size_t n_integers = 0; static void test_destroy(int *b) { (*b)++; } +static void test_strings_destroy(char **array, size_t n) { + assert_se(n == 3); + assert_se(strv_equal(array, STRV_MAKE("a", "bbb", "ccc"))); + + strv_free(array); +} + +static void test_integers_destroy(int *array, size_t n) { + assert_se(n == 10); + + for (size_t i = 0; i < n; i++) + assert_se(array[i] == (int)(i * i)); + + free(array); +} + STATIC_DESTRUCTOR_REGISTER(foo, test_destroy); STATIC_DESTRUCTOR_REGISTER(bar, test_destroy); STATIC_DESTRUCTOR_REGISTER(bar, test_destroy); @@ -20,15 +41,27 @@ STATIC_DESTRUCTOR_REGISTER(baz, test_destroy); STATIC_DESTRUCTOR_REGISTER(baz, test_destroy); STATIC_DESTRUCTOR_REGISTER(baz, test_destroy); STATIC_DESTRUCTOR_REGISTER(memory, freep); +STATIC_ARRAY_DESTRUCTOR_REGISTER(strings, n_strings, test_strings_destroy); +STATIC_ARRAY_DESTRUCTOR_REGISTER(integers, n_integers, test_integers_destroy); TEST(static_destruct) { - assert_se(memory = strdup("hallo")); - assert_se(foo == 0 && bar == 0 && baz == 0); - static_destruct(); - assert_se(foo == 1 && bar == 2 && baz == 3); + assert_se(memory = strdup("hallo")); + assert_se(strings = strv_new("a", "bbb", "ccc")); + n_strings = strv_length(strings); + n_integers = 10; + assert_se(integers = new(int, n_integers)); + for (size_t i = 0; i < n_integers; i++) + integers[i] = i * i; - assert_se(memory == NULL); + static_destruct(); + + assert_se(foo == 1 && bar == 2 && baz == 3); + assert_se(!memory); + assert_se(!strings); + assert_se(n_strings == 0); + assert_se(!integers); + assert_se(n_integers == 0); } DEFINE_TEST_MAIN(LOG_INFO);