From mboxrd@z Thu Jan  1 00:00:00 1970
From: cgit at cryptocrack.de (Lukas Fleischer)
Date: Mon, 4 Mar 2013 00:41:18 +0100
Subject: [PATCH] ui-snapshot.c: Terminate cgit_snapshot_formats[] properly
In-Reply-To: <20130303225516.GM7738@serenity.lan>
References: <1362348541-26785-1-git-send-email-cgit@cryptocrack.de>
 <20130303225516.GM7738@serenity.lan>
Message-ID: <20130303234118.GA8215@blizzard>

On Sun, Mar 03, 2013 at 10:55:16PM +0000, John Keeping wrote:
> On Sun, Mar 03, 2013 at 11:09:01PM +0100, Lukas Fleischer wrote:
> > Explicitly set the suffix field of the terminating format entry to 0.
> > This fixes a GCC warning seen with "-Wmissing-field-initializers".
> > 
> > Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> > ---
> >  ui-snapshot.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/ui-snapshot.c b/ui-snapshot.c
> > index 47432bd..e740645 100644
> > --- a/ui-snapshot.c
> > +++ b/ui-snapshot.c
> > @@ -48,7 +48,7 @@ const struct cgit_snapshot_format cgit_snapshot_formats[] = {
> >  	{ ".tar.bz2", "application/x-bzip2", write_tar_bzip2_archive, 0x04 },
> >  	{ ".tar", "application/x-tar", write_tar_archive, 0x08 },
> >  	{ ".tar.xz", "application/x-xz", write_tar_xz_archive, 0x10 },
> > -	{}
> > +	{ 0 }
> 
> I'm mildly against this - we're not fixing an issue that's been found with some
> specific compiler, the meaning of both versions is the same and
> -Wmissing-field-initializers isn't in -Wall.  It feels like a warning that was
> added for people following some overly prescriptive coding standard, not a
> warning that's actually useful.

Looking at the C89/C99 standards (section 6.7.8), it doesn't look like
using "{}" as initializer is valid at all. In the C++ standard, a
special grammar rule was added to allow for using "{}" as initializer
(see section 8.5). I guess we're rather fixing a "real" bug (nothing of
practical importance, but definitely not a compiler-related issue) here.
Maybe the commit message should be updated to reflect this.

> 
> [Also, I'm surprised this is sufficient to squelch the warning given the
> description of -Wmissing-field-initializers in gcc(1):
> 
>    Warn if a structure's initializer has some fields missing.  For example,
>    the following code would cause such a warning, because "x.h" is implicitly
>    zero:
> 
>        struct s { int f, g, h; };
>        struct s x = { 3, 4 };
> ]

Yes, it's a bit weird. My guess is that if you're doing something like
"struct s x = { 3, 4 };" in the example above, you most likely forgot to
set the last field, whereas if you're doing something like "struct s x =
{ 0 };" you're explicitly (and deliberately) initializing everything to
zeros. The compiler is kind of guessing whether the missing members are
skipped on purpose or not.

> 
> >  };
> >  
> >  static const struct cgit_snapshot_format *get_format(const char *filename)
> > -- 
> > 1.8.2.rc0.247.g811e0c0