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

From: Taylor Blau <me@ttaylorr.com>
Date: Apr 15, 2025, 19:47 UTC
Message-ID: <Z/637cMz7AGVXokX@nand.local>
In-Reply-To: <CABPp-BGH=GRAo2Vy8sia75p=2Uoza0kwTzoy9FKKxE5jDFfGXg@mail.gmail.com>

On Mon, Apr 14, 2025 at 07:57:59PM -0700, Elijah Newren wrote:

Show 13 quoted lines

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

For (a), I wouldn't say that the _INIT macros are the potentially-buggy component, but rather the use of a designated initializer in the presence of an _INIT macro that initializes the struct to something other than all zeros.

I don't think there is anything specific to the object_info structure here, which suggests (d), but I don't think that the warning is unnecessary, it's just overly-specific. I think we should have a convention in CodingGuidelines that forbids designated initializers in structures with non-zero _INIT macros that don't otherwise have a comment about their correctness.

But I think there is some discussion to be had there, and I want to disentangle that from this series. I'm going to drop this and the following patch from this series and split them out so we can discuss them more thoroughly while letting this series move ahead.

Thanks, Taylor