Re: [PATCH] difftool.c: mark a file-local symbol with static

From: Jeff King <peff@peff.net>
Date: Jan 24, 2017, 14:23 UTC
Message-ID: <20170124142346.u3d7l6772mtkgpcf@sigill.intra.peff.net>
In-Reply-To: <20170122052608.tpr5pihfgafhoynj@gmail.com>

mn Sat, Jan 21, 2017 at 09:26:08PM -0800, David Aguilar wrote:

Show 16 quoted lines

> > > An obvious second
> > > best option would be to drop -Wall from the "everybody" CFLAGS and
> > > move it to DEVELOPER_CFLAGS instead.
> > 
> > Yeah, though that doesn't help the example above.
> > 
> > As ugly as warning("%s", "") is, I think it may be the thing that annoys
> > the smallest number of people.
> > 
> > -Peff
> 
> How about using warning(" ") instead?
> 
> For difftool.c specifically, the following is a fine solution,
> and doesn't require that we change our warning flags just for
> this one file.

I dunno. As ugly as the "%s" thing is in the source, at least it doesn't change the output. Not that an extra space is the end of the world, but it seems like it's letting the problem escape from the source code.

Do people still care about resolving this? -Wno-format-zero-length is in the DEVELOPER options. It wasn't clear to me if that was sufficient, or if we're going to get a bunch of reports from people that need to be directed to the right compiler options.

The problematic code just hit 'next', so I suppose we'll see soon (OTOH, the real test is when it get shipped as part of a release).

-Peff