[PATCH] ui-snapshot.c: Terminate cgit_snapshot_formats[] properly

[John Keeping]

On Mon, Mar 04, 2013 at 12:41:18AM +0100, Lukas Fleischer wrote:
> 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.

Interesting.  A careful reading agrees with this, although I haven't
come across a compiler that would treat these differently and it's
slightly strange that you can't keep reducing the "unspecified members
take a default value" case until the list is empty.

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