Re: [PATCH v3 4/8] reftable/stack: fix compiler warning due to missing braces

From: Justin Tobler <jltobler@gmail.com>
Date: Aug 12, 2025, 16:51 UTC
Message-ID: <puevci6dx6qf235usn7sxi52hlt2n5dcbe4gb2r3l4yekvr7mq@qp7l4nwkbnvg>
In-Reply-To: <20250812-pks-reftable-fixes-for-libgit2-v3-4-cf3b2267867e@pks.im>

On 25/08/12 11:54AM, Patrick Steinhardt wrote:

Show 38 quoted lines

> 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@sunshineco.com/T/
> 
> Suggested-by: Carlo Arenas <carenas@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  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.

Show 9 quoted lines

> -
>  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.

Show 29 quoted lines

>  	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.

Show 13 quoted lines

>  	err = reftable_stack_init_addition(*dest, st, flags);
>  	if (err) {
>  		reftable_free(*dest);
>  		*dest = NULL;
>  	}
> +
>  	return err;
>  }
>  
> 
> -- 
> 2.51.0.rc1.163.g2494970778.dirty
>