Re: [PATCH v2 2/8] object-store-ll.h: add note about designated initializers

[Elijah Newren <newren@gmail.com>]

On Mon, Apr 14, 2025 at 1:06 PM Taylor Blau <me@ttaylorr.com> wrote:
>
> The following commit will use a designated initializer to initialize a
> 'struct object_info'. This obviously depends on having the rest of the
> fields having a default value of zero, since unspecified fields in a
> designated initializer are zero'd out.
>
> Before writing that designated initializer, I wondered if there were
> other spots that also use designated initializers to set up object_info
> structs, and there are a handful.
>
> To prevent potential breakage against future object_info changes that
> would introduce/change a field to have a non-zero default value, note
> this dependency in a comment near the OBJECT_INFO_INIT macro.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  object-store-ll.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/object-store-ll.h b/object-store-ll.h
> index cd3bd5bd99..7ff180d7f2 100644
> --- a/object-store-ll.h
> +++ b/object-store-ll.h
> @@ -337,6 +337,14 @@ struct object_info {
>  /*
>   * Initializer for a "struct object_info" that wants no items. You may
>   * also memset() the memory to all-zeroes.
> + *
> + * NOTE: callers expect the initial value of an object_info struct to
> + * be zero'd out. Designated initializers like
> + *
> + *     struct object_info oi = { .sizep = &sz };
> + *
> + * depend on this behavior, so consider strongly before adding new
> + * fields that have a non-zero default value.
>   */
>  #define OBJECT_INFO_INIT { 0 }

There are 46 #define'd designated initializers in the code base, from
DIR_INIT to OIDMAP_INIT and everything in-between.  The logic used in
your comment to suggest not using an all-zeroes initializer doesn't
seem to depend in any way on something specific to object_info, yet
none of those other 46 cases in my quick scanning have such a warning.
And 29 of the 46 define some kind of initial value for some fields
instead of using all zeroes.  That would suggest that one of the
following is true: (a) those 29 cases are buggy and shouldn't be doing
that, (b) those 29 are all special cases someone has thought through
carefully but perhaps someone should add the same warning you have
here to those 29 other cases to avoid uncarefully thought cases from
being added, (c) there is something specific about object_info that
you didn't call out here, or (d) this warning you add is unnecessary.