Re: Fixing the warning about warning(""); was: Re: [PATCH] difftool.c: mark a file-local symbol with static
- From
Jeff King <peff@peff.net>
- Date
- Jan 25, 2017, 18:35 UTC
- Message-ID
- <20170125183542.pe5qolexqqx6jhsi@sigill.intra.peff.net>
- In-Reply-To
- <alpine.DEB.2.20.1701251135090.3469@virtualbox>
On Wed, Jan 25, 2017 at 11:36:50AM +0100, Johannes Schindelin wrote:
Show 14 quoted lines
> > Gross, but at least it's self documenting. :)
> >
> > I guess a less horrible version of that is:
> >
> > static inline warning_blank_line(void)
> > {
> > warning("%s", "");
> > }
> >
> > We'd potentially need a matching one for error(), but at last it avoids
> > macro trickery.
>
> I fail to see how this function, or this definition, makes the code better
> than simply calling `warning("%s", "");` and be done with it.The only advantage is that it is self-documenting, so somebody does not come through later and convert ("%s", "") back to (""). We could also write a comment. But I am happy if we simply catch it in review (or preferably the person is clueful enough to read the output of git-blame and see why it is that way in the first place).
So maybe:
-- >8 -- Subject: [PATCH] difftool: hack around -Wzero-length-format warning
Building with "gcc -Wall" will complain that the format in:
warning("")is empty. Which is true, but the warning is over-eager. We are calling the function for its side effect of printing "warning:", even with an empty string.
Our DEVELOPER Makefile knob disables the warning, but not everybody uses it. Let's silence the warning in the code so that nobody reports it or tries to "fix" it.
Signed-off-by: Jeff King <peff@peff.net> --- builtin/difftool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c index 42ad9e804..b5e85ab07 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -567,7 +567,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, warning(_("both files modified: '%s' and '%s'."), wtdir.buf, rdir.buf); warning(_("working tree file has been left.")); - warning(""); + warning("%s", ""); err = 1; } else if (unlink(wtdir.buf) || copy_file(wtdir.buf, rdir.buf, st.st_mode))
-- 2.11.0.840.gd37c5973a