Re: [PATCH 1/5] *.h: move some *_INIT to designated initializers
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Jul 1, 2021, 15:00 UTC
- Message-ID
- <CAN0heSpsg=bwX9gbYfeWYqUbuen-zArR69FWxNmdtcQW_RubzA@mail.gmail.com>
- In-Reply-To
- <patch-1.5-9a92e7c216f-20210701T104855Z-avarab@gmail.com>
On Thu, 1 Jul 2021 at 12:54, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 5 quoted lines
> > 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.
> -#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.
> -#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".
Martin