On Thu, Jul 01 2021, Martin Ågren wrote:

On Thu, 1 Jul 2021 at 12:54, Ævar Arnfjörð Bjarmason [off-list ref] wrote:

quoted

Move *_INIT macros I'll use in a subsequent commits to designated
initializers. This isn't required for those follow-up changes, but
since I'm changing things in this are let's use the modern pattern
over the old one while we're at it.

s/are/area/, maybe even s/area/area,/ on top.

Will fix.

quoted

-#define CREDENTIAL_INIT { STRING_LIST_INIT_DUP }
+#define CREDENTIAL_INIT { \
+       .helpers = STRING_LIST_INIT_DUP, \
+}

I've verified that all of these designated initializers assign the exact
same fields before and after this commit, e.g., `helpers` actually is
what comes first in the struct.

Thanks!

quoted

-#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL }
-#define STRING_LIST_INIT_DUP   { NULL, 0, 0, 1, NULL }
+#define STRING_LIST_INIT_NODUP { 0 }
+#define STRING_LIST_INIT_DUP   { .strdup_strings = 1 }

This "NODUP" one is a bit of an odd case though. You don't actually
change it to use designated initializers, as the patch says. Instead you
change it in a slightly different way. In a sense, you assign the
literal zero not to strdup_strings, but to the first field, which is a
pointer, where I would have expected NULL.

I think there's been some recent-ish "we can assign `{ 0 }` even if the
first field is a pointer, because it's idiomatic and works out fine"
even if assigning 0 to a pointer looks a bit odd. So it might not be
wrong as such, but it doesn't match what the commit message claims, and
I think it would be more clear to use `{ .strdup_strings = 0 }` here:
We're explicitly interested in *not* duplicating the strings. It's not
just some default "let's leave everything as zero/NULL".

Yes I'm really conflating a few different things here:

 1. Whether to use designated initializers.

 2. Whether to skip explicit initialization, i.e. not enumerate
    everything, and have the ones not mentioned initializes as if they
    had static storage duration.

 3. Using the 0 null pointer constant instead of NULL while doing #2.

 4. Using our in-codebase idiom of only setting any boolean flags during
    initialization if we're setting them to true, 

So I think in this case just keeping it as "{ 0 }" makes the most
sense. Even better would be just a "{}", but that's a GNU-only
extension.

I think "{ NULL }" is just a distraction, i.e. it's better to use "{ 0
}" consistently everywhere. C guarantees the same-ness of the two, and
the point is to initialize the struct as if though it were done with a
memset() to 0, not mentally keep track of whether the first member is an
int, pointer or char * (in one other case we use '\0').