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, 22:01 UTC
- Message-ID
- <20170125220101.et67ebkumsqosaku@sigill.intra.peff.net>
- In-Reply-To
- <xmqq7f5iakxw.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 25, 2017 at 01:28:27PM -0800, Junio C Hamano wrote:
Show 16 quoted lines
> Jeff King <peff@peff.net> writes:
>
> > 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).
>
> And the last sentence unfortunatly does not reflect reality.
>
> I would prefer something self-documenting, like your wrapper with a
> comment. Then somebody who is looking at the implementation of
> warning_blank_line() will not get tempted to turn "%s", "" into ""
> because of the comment. And somebody who is looking at the callsite
> of warning_blank_line() will think twice before suggesting to turn
> it into warning("").I am OK with it either way. I was mostly responding to Dscho's complaint, and I would just like to get this resolved so we never have to revisit it again. :)
> In any case, the patch is a minimum effort band-aid that lets us > punt on the whole issue for now, so I'll queue it as-is.
Here's one other option that I came across. Pragmas feel gross, but I think it will behave as we want, and it doesn't require cooperation from the callsites at all.
-- >8 -- Subject: [PATCH] disable -Wzero-length-format via #pragma
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.
We disable this warning already with the DEVELOPER Makefile knob. But we can't unconditionally add -Wno-format-zero-length to the normal CFLAGS variable, because not all compilers will understand it. So we may get reports about the warning from non-developer users who compile with our default of -Wall.
Instead, we can disable the warning using a gcc-specific #pragma. This should be ignored by non-gcc compilers, and do what we want for gcc.
I tested also with clang, which often implements gcc compatible extensions like this. Clang does not generate the warning in the first place, but also does not complain about our pragma.
Signed-off-by: Jeff King <peff@peff.net> --- git-compat-util.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h index 325950426..a6558930d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -413,6 +413,8 @@ extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2))); extern void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2))); +#pragma GCC diagnostic ignored "-Wformat-zero-length" + #ifndef NO_OPENSSL #ifdef APPLE_COMMON_CRYPTO #include "compat/apple-common-crypto.h"
-- 2.11.0.840.gd37c5973a