From: Zbigniew Jędrzejewski-Szmek Date: Sat, 16 May 2026 16:32:14 +0000 (+0200) Subject: tree-wide: get rid of most uses of ALIGN_PTR X-Git-Tag: v261-rc1~135^2~10 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=cf88903637;p=thirdparty%2Fsystemd.git tree-wide: get rid of most uses of ALIGN_PTR This bothered me for a while, but I didn't think too much about it and just copied the existing usage pattern. But it really doesn't make sense. We expect the compiler to align the section properly. But if it didn't align it, applying alignment after the fact would just cause our pointer to point to the middle of the structure. That'd be even worse than a misaligned pointer. Similarly, when doing pointer arithmetic, p++ should really result in a value with the appropriate alignment. This is the basic principle of C pointer addition. So we really shouldn't try to adjust the pointer ourselves. At most, we can assert that it is indeed aligned in tests. --- diff --git a/src/basic/static-destruct.c b/src/basic/static-destruct.c index c39a1b38164..442f5fe7ba4 100644 --- a/src/basic/static-destruct.c +++ b/src/basic/static-destruct.c @@ -1,13 +1,12 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include "memory-util.h" #include "static-destruct.h" void static_destruct_impl(const StaticDestructor *start, const StaticDestructor *end) { if (!start) return; - for (const StaticDestructor *d = ALIGN_PTR(start); d < end; d = ALIGN_PTR(d + 1)) + for (const StaticDestructor *d = start; d < end; d++) switch (d->type) { case STATIC_DESTRUCTOR_SIMPLE: d->simple.destroy(d->simple.data); diff --git a/src/basic/static-destruct.h b/src/basic/static-destruct.h index 00087ad779e..fdc4c7c41ec 100644 --- a/src/basic/static-destruct.h +++ b/src/basic/static-destruct.h @@ -44,6 +44,8 @@ typedef struct StaticDestructor { }; } StaticDestructor; +assert_cc(sizeof(StaticDestructor) % sizeof(void*) == 0); + #define STATIC_DESTRUCTOR_REGISTER(variable, func) \ _STATIC_DESTRUCTOR_REGISTER(UNIQ, variable, func) diff --git a/src/libsystemd/sd-bus/bus-error.c b/src/libsystemd/sd-bus/bus-error.c index be64e4a6c8b..bee843e3380 100644 --- a/src/libsystemd/sd-bus/bus-error.c +++ b/src/libsystemd/sd-bus/bus-error.c @@ -83,22 +83,17 @@ static int bus_error_name_to_errno(const char *name) { } } - const sd_bus_error_map *elf_map = ALIGN_PTR(__start_SYSTEMD_BUS_ERROR_MAP); - while (elf_map < __stop_SYSTEMD_BUS_ERROR_MAP) { + assert_cc(sizeof(sd_bus_error_map) % sizeof(void*) == 0); + + for (const sd_bus_error_map *m = __start_SYSTEMD_BUS_ERROR_MAP; m < __stop_SYSTEMD_BUS_ERROR_MAP; m++) { /* For magic ELF error maps, the end marker might appear in the middle of things, since - * multiple maps might appear in the same section. Hence, let's skip over it, but realign - * the pointer to the next 8 byte boundary, which is the selected alignment for the arrays. */ - if (elf_map->code == BUS_ERROR_MAP_END_MARKER) { - elf_map = ALIGN_PTR(elf_map + 1); - continue; - } + * multiple maps might appear in the same section. Skip over it. */ - if (streq(elf_map->name, name)) { - assert(elf_map->code > 0); - return elf_map->code; + if (m->code != BUS_ERROR_MAP_END_MARKER && + streq(m->name, name)) { + assert(m->code > 0); + return m->code; } - - elf_map++; } return EIO; diff --git a/src/libsystemd/sd-bus/test-bus-error.c b/src/libsystemd/sd-bus/test-bus-error.c index 3f89a907eb6..a91a014a38c 100644 --- a/src/libsystemd/sd-bus/test-bus-error.c +++ b/src/libsystemd/sd-bus/test-bus-error.c @@ -128,19 +128,12 @@ extern const sd_bus_error_map __start_SYSTEMD_BUS_ERROR_MAP[]; extern const sd_bus_error_map __stop_SYSTEMD_BUS_ERROR_MAP[]; static int dump_mapping_table(void) { - const sd_bus_error_map *m; - printf("----- errno mappings ------\n"); - m = ALIGN_PTR(__start_SYSTEMD_BUS_ERROR_MAP); - while (m < __stop_SYSTEMD_BUS_ERROR_MAP) { - - if (m->code == BUS_ERROR_MAP_END_MARKER) { - m = ALIGN_PTR(m + 1); - continue; - } + for (const sd_bus_error_map *m = __start_SYSTEMD_BUS_ERROR_MAP; m < __stop_SYSTEMD_BUS_ERROR_MAP; m++) { + assert((uintptr_t) m % sizeof(void*) == 0); - printf("%s -> %i/%s\n", strna(m->name), m->code, ERRNO_NAME(m->code)); - m++; + if (m->code != BUS_ERROR_MAP_END_MARKER) + printf("%s -> %i/%s\n", strna(m->name), m->code, ERRNO_NAME(m->code)); } printf("---------------------------\n"); diff --git a/src/shared/options.h b/src/shared/options.h index c2283dc4063..fd3ab008d36 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once -#include "memory-util.h" #include "shared-forward.h" /* Option namespace/group explanation: @@ -240,10 +239,10 @@ int option_parse( /* Iterate over options. Don't forget to handle errors (negative c)! */ #define FOREACH_OPTION(c, state) \ - for (int c; (c = option_parse(ALIGN_PTR(__start_SYSTEMD_OPTIONS), __stop_SYSTEMD_OPTIONS, state)) != 0; ) + for (int c; (c = option_parse(__start_SYSTEMD_OPTIONS, __stop_SYSTEMD_OPTIONS, state)) != 0; ) #define FOREACH_OPTION_OR_RETURN(c, state) \ - for (int c; (c = option_parse(ALIGN_PTR(__start_SYSTEMD_OPTIONS), __stop_SYSTEMD_OPTIONS, state)) != 0; ) \ + for (int c; (c = option_parse(__start_SYSTEMD_OPTIONS, __stop_SYSTEMD_OPTIONS, state)) != 0; ) \ if (c < 0) \ return c; \ else @@ -268,7 +267,7 @@ int _option_parser_get_help_table_full( const char *group, Table **ret); #define option_parser_get_help_table_full(namespace, group, ret) \ - _option_parser_get_help_table_full(ALIGN_PTR(__start_SYSTEMD_OPTIONS), __stop_SYSTEMD_OPTIONS, namespace, group, ret) + _option_parser_get_help_table_full(__start_SYSTEMD_OPTIONS, __stop_SYSTEMD_OPTIONS, namespace, group, ret) #define option_parser_get_help_table_ns(ns, ret) \ option_parser_get_help_table_full(ns, /* group= */ NULL, ret) #define option_parser_get_help_table_group(group, ret) \ diff --git a/src/shared/tests.c b/src/shared/tests.c index 84945343a46..176c7fb13f3 100644 --- a/src/shared/tests.c +++ b/src/shared/tests.c @@ -380,20 +380,18 @@ int run_test_table(const TestFunc *start, const TestFunc *end) { _cleanup_strv_free_ char **tests = NULL; int r = EXIT_SUCCESS; bool ran = false; - const char *e; if (!start) return r; - e = getenv("TESTFUNCS"); + const char *e = getenv("TESTFUNCS"); if (e) { r = strv_split_full(&tests, e, ":", EXTRACT_DONT_COALESCE_SEPARATORS); if (r < 0) return log_error_errno(r, "Failed to parse $TESTFUNCS: %m"); } - for (const TestFunc *t = ALIGN_PTR(start); t + 1 <= end; t = ALIGN_PTR(t + 1)) { - + for (const TestFunc *t = start; t + 1 <= end; t++) { if (tests && !strv_contains(tests, t->name)) continue; diff --git a/src/shared/verbs.c b/src/shared/verbs.c index aab24c06819..bd665fc7fa2 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -66,6 +66,9 @@ static bool verb_is_metadata(const Verb *verb) { const Verb* verbs_find_verb(const char *name, const Verb verbs[], const Verb verbs_end[]) { assert(verbs); + assert(verbs_end > verbs); + assert((uintptr_t) verbs % sizeof(void*) == 0); + assert(verbs[0].verb); for (const Verb *verb = verbs; verb < verbs_end; verb++) { if (verb_is_metadata(verb)) @@ -84,6 +87,7 @@ int _dispatch_verb_with_args(char **args, const Verb verbs[], const Verb verbs_e assert(verbs); assert(verbs_end > verbs); + assert((uintptr_t) verbs % sizeof(void*) == 0); assert(verbs[0].verb); const char *name = args ? args[0] : NULL; @@ -150,6 +154,7 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { /* getopt wrapper for _dispatch_verb_with_args. * TBD: remove this function when all programs with verbs have been converted. */ + assert((uintptr_t) verbs % sizeof(void*) == 0); assert(argc >= 0); assert(argv); assert(argc >= optind); @@ -234,6 +239,9 @@ int _verbs_get_help_table( Table **ret) { int r; + assert(verbs); + assert(verbs_end > verbs); + assert((uintptr_t) verbs % sizeof(void*) == 0); assert(ret); _cleanup_(table_unrefp) Table *table = table_new("verb", "help"); diff --git a/src/shared/verbs.h b/src/shared/verbs.h index e071cc5d89a..a5d210c8931 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -81,7 +81,7 @@ const Verb* verbs_find_verb(const char *name, const Verb verbs[], const Verb ver int _dispatch_verb_with_args(char **args, const Verb verbs[], const Verb verbs_end[], void *userdata); #define dispatch_verb_with_args(args, userdata) \ - _dispatch_verb_with_args(args, ALIGN_PTR(__start_SYSTEMD_VERBS), __stop_SYSTEMD_VERBS, userdata) + _dispatch_verb_with_args(args, __start_SYSTEMD_VERBS, __stop_SYSTEMD_VERBS, userdata) int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata); @@ -91,7 +91,7 @@ int _verbs_get_help_table( const char *group, Table **ret); #define verbs_get_help_table_group(group, ret) \ - _verbs_get_help_table(ALIGN_PTR(__start_SYSTEMD_VERBS), __stop_SYSTEMD_VERBS, group, ret) + _verbs_get_help_table(__start_SYSTEMD_VERBS, __stop_SYSTEMD_VERBS, group, ret) #define verbs_get_help_table(ret) \ verbs_get_help_table_group(/* group= */ NULL, ret) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d5473cdc0af..6ea4e50fb4a 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1112,9 +1112,9 @@ VERB_SCOPE(, verb_start, "condrestart", NULL, 2, VERB_ANY, VERB_SCOPE(, verb_is_active, "check", NULL, 2, VERB_ANY, VERB_ONLINE_ONLY, /* help= */ NULL); /* deprecated alias of is-active */ int systemctl_main(char **args) { - const Verb *verb = verbs_find_verb(args[0], - ALIGN_PTR(__start_SYSTEMD_VERBS), - __stop_SYSTEMD_VERBS); + assert((uintptr_t) __start_SYSTEMD_VERBS % sizeof(void*) == 0); + const Verb *verb = verbs_find_verb(args[0], __start_SYSTEMD_VERBS, __stop_SYSTEMD_VERBS); + if (verb && (verb->flags & VERB_ONLINE_ONLY) && arg_root) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Verb '%s' cannot be used with --root= or --image=.",