On 25/08/12 11:54AM, Patrick Steinhardt wrote:
> While perfectly legal, older compiler toolchains complain when
> zero-initializing structs that contain nested structs with `{0}`:
>
> /home/libgit2/source/deps/reftable/stack.c:862:35: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
> struct reftable_addition empty = REFTABLE_ADDITION_INIT;
> ^~~~~~~~~~~~~~~~~~~~~~
> /home/libgit2/source/deps/reftable/stack.c:707:33: note: expanded from macro 'REFTABLE_ADDITION_INIT'
> #define REFTABLE_ADDITION_INIT {0}
> ^
>
> We had the discussion around whether or not we want to handle such bogus
> compiler errors in the past already [1]. Back then we basically decided
> that we do not care about such old-and-buggy compilers, so while we
> could fix the issue by using `{{0}}` instead this is not the preferred
> way to handle this in the Git codebase.
>
> We have an easier fix though: we can just drop the macro altogether and
> handle initialization of the struct in `reftable_stack_addition_init()`.
> Callers are expected to call this function already, so this change even
> simplifies the calling convention.
>
> [1]: https://lore.kernel.org/git/20220710081135.74964-1-sunshine@xxxxxxxxxxxxxx/T/
>
> Suggested-by: Carlo Arenas <carenas@xxxxxxxxx>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> reftable/stack.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/reftable/stack.c b/reftable/stack.c
> index ed80710572..9db90cf4ed 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -664,8 +664,6 @@ struct reftable_addition {
> uint64_t next_update_index;
> };
>
> -#define REFTABLE_ADDITION_INIT {0}
It looks like there are only two places where this macro gets used.
Being that `reftable_stack_init_addition()` is always expected to be
called, deferring initialization of the structure to that point seems
sensible.
> -
> static void reftable_addition_close(struct reftable_addition *add)
> {
> struct reftable_buf nm = REFTABLE_BUF_INIT;
> @@ -693,6 +691,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
> struct reftable_buf lock_file_name = REFTABLE_BUF_INIT;
> int err;
>
> + memset(add, 0, sizeof(*add));
Looks good.
> add->stack = st;
>
> err = flock_acquire(&add->tables_list_lock, st->list_file,
> @@ -739,8 +738,10 @@ static int stack_try_add(struct reftable_stack *st,
> void *arg),
> void *arg)
> {
> - struct reftable_addition add = REFTABLE_ADDITION_INIT;
> - int err = reftable_stack_init_addition(&add, st, 0);
> + struct reftable_addition add;
> + int err;
> +
> + err = reftable_stack_init_addition(&add, st, 0);
> if (err < 0)
> goto done;
>
> @@ -866,19 +867,18 @@ int reftable_stack_new_addition(struct reftable_addition **dest,
> struct reftable_stack *st,
> unsigned int flags)
> {
> - int err = 0;
> - struct reftable_addition empty = REFTABLE_ADDITION_INIT;
> + int err;
>
> REFTABLE_CALLOC_ARRAY(*dest, 1);
> if (!*dest)
> return REFTABLE_OUT_OF_MEMORY_ERROR;
>
> - **dest = empty;
Now resetting the `reftable_addition` is handled by
`reftable_stack_init_addition` automatically, which is nicer IMO.
> err = reftable_stack_init_addition(*dest, st, flags);
> if (err) {
> reftable_free(*dest);
> *dest = NULL;
> }
> +
> return err;
> }
>
>
> --
> 2.51.0.rc1.163.g2494970778.dirty
>
