]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
tree-wide: get rid of most uses of ALIGN_PTR
authorZbigniew Jędrzejewski-Szmek <zbyszek@amutable.com>
Sat, 16 May 2026 16:32:14 +0000 (18:32 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@amutable.com>
Sat, 16 May 2026 16:32:14 +0000 (18:32 +0200)
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.

src/basic/static-destruct.c
src/basic/static-destruct.h
src/libsystemd/sd-bus/bus-error.c
src/libsystemd/sd-bus/test-bus-error.c
src/shared/options.h
src/shared/tests.c
src/shared/verbs.c
src/shared/verbs.h
src/systemctl/systemctl.c

index c39a1b381649ad9147de284790f02bf1da6e6c1b..442f5fe7ba4e8e37f82bd54b1d3e4e4feb090f19 100644 (file)
@@ -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);
index 00087ad779e0a321b920ca777535f60c1e1a8373..fdc4c7c41ec37506e83815b812ffea826f31ee73 100644 (file)
@@ -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)
 
index be64e4a6c8bc023ad974c8a0f2887e0d16bb7896..bee843e33806de24be5cfeee2dd174b648af53d2 100644 (file)
@@ -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;
index 3f89a907eb654f6c0a861dafaf1892c7bbcccbbf..a91a014a38ca9bb50708cdb78f2f00adb2524c88 100644 (file)
@@ -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");
 
index c2283dc4063f13e4bf0d3c190207543fd294b70e..fd3ab008d36f281c48af624bfba46d7c39a0222d 100644 (file)
@@ -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)                  \
index 84945343a46505b54383208b44e4c4e02d6cfdbf..176c7fb13f3737c123547cf14b08d820cbeff636 100644 (file)
@@ -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;
 
index aab24c068199a58eb4961c4723ea199ff957e617..bd665fc7fa2446e92a94f088506f758f21ce573f 100644 (file)
@@ -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");
index e071cc5d89aa93df39e503c012a178461622f845..a5d210c89311f17ebbc6f1f24cfe940d1b05e44b 100644 (file)
@@ -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)
 
index d5473cdc0af4b7d14bcc933585ee1507f26118f2..6ea4e50fb4a80381f6dfc2174e5b6622dd5ff562 100644 (file)
@@ -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=.",