Re: [PATCH 1/5] *.h: move some *_INIT to designated initializers

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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

> On Thu, 1 Jul 2021 at 12:54, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>>
>> 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.

>> -#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!

>> -#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').