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

From: Jeff King <peff@peff.net>
To: David Aguilar <davvid@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Ramsay Jones <ramsay@ramsayjones.plus.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	GIT Mailing-list <git@vger.kernel.org>
Subject: Re: [PATCH] difftool.c: mark a file-local symbol with static
Date: Tue, 24 Jan 2017 09:23:46 -0500	[thread overview]
Message-ID: <20170124142346.u3d7l6772mtkgpcf@sigill.intra.peff.net> (raw)
In-Reply-To: <20170122052608.tpr5pihfgafhoynj@gmail.com>

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

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