Re: [PATCH 0/5] cleaning up read_object() family of functions

From: Jeff King <peff@peff.net>
Date: Jan 12, 2023, 16:16 UTC
Message-ID: <Y8AyTE3OS7HCAzKH@coredump.intra.peff.net>
In-Reply-To: <230112.86fscg2jbm.gmgdl@evledraar.gmail.com>

On Thu, Jan 12, 2023 at 10:21:46AM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 16 quoted lines

> I agree that it's probably not worth it here, but I think you're just
> tying yourself in knots in trying to define these macros in terms of
> each other. This sort of thing will work if you just do:
> 	
> 	diff --git a/object-store.h b/object-store.h
> 	index e894cee61ba..bfcd2482dc5 100644
> 	--- a/object-store.h
> 	+++ b/object-store.h
> 	@@ -418,8 +418,8 @@ struct object_info {
> 	  * Initializer for a "struct object_info" that wants no items. You may
> 	  * also memset() the memory to all-zeroes.
> 	  */
> 	-#define OBJECT_INFO(...) { 0, __VA_ARGS__ }
> 	-#define OBJECT_INFO_INIT OBJECT_INFO()
> 	+#define OBJECT_INFO_INIT { 0 }
> 	+#define OBJECT_INFO(...) { __VA_ARGS__ }

Right, that works because the initializer is just "0", which the compiler can do for us implicitly. I agree it works here to omit, but as a general solution, it doesn't.

Show 7 quoted lines

> Which is just a twist on René's suggestion from [1], i.e.:
> 
> 	#define CHILD_PROCESS_INIT_EX(...) { .args = STRVEC_INIT, __VA_ARGS__ }
>
> In that case we always need to rely on the "args" being init'd, and the
> GCC warning you note is a feature, its initialization is "private", and
> you should never override it.

Right, and it works here because you'd never want to init .args to anything else (which I think is what you mean by "private"). But in the general case the defaults can't set something that the caller might want to override, because the compiler's warning doesn't know the difference between "override" and "oops, you specified this twice".

It's mostly a non-issue because we tend to prefer 0-initialization when possible, but I think as a general technique this is probably opening a can of worms for little benefit.

-Peff