[Bug c/47901] New: -Wall should not imply -Wformat-zero-length by default

[Bug c/47901] New: -Wall should not imply -Wformat-zero-length by default

[anders at kaseorg dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

           Summary: -Wall should not imply -Wformat-zero-length by default
           Product: gcc
           Version: 4.5.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: anders@kaseorg.com


There is nothing wrong with printf("") or custom_printf_like_function(foo, "").
 There are plenty of reasons to write such code: for example, when using a
macro that sometimes expands to the empty string, or if
custom_printf_like_function does something else with foo in addition to using
the format string, or if the string is used for something other than printing
and you really just want the empty string there.

gcc/c-format.c contains a comment that says “If the format is an empty string,
this should be counted similarly to the case of extra format arguments.”  But
those are not actually similar.  Extra format arguments clearly indicate that
the code was not written as intended, because the extra arguments could be
removed with no effects; but a zero-length string is there for a reason and
can’t just be removed.

When GCC complains about legitimate empty format strings as soon as -Wall is
turned on, it only leads to frustration with -Wall.  This leads some users to
give up on -Wall altogether, and thus to miss many of the real problems -Wall
would find; and leads others to come up with brain-damaged workarounds like
adding a useless space:
http://git.kernel.org/linus/6f131ce1dfa9b283ddc212df42b015d152c670a5

Can we change -Wformat-zero-length to default to off unless explicitly
requested?

See also:
http://gcc.gnu.org/ml/gcc-patches/2002-05/msg01469.html
http://ewx.livejournal.com/517490.html
http://www.mail-archive.com/qemu-devel@nongnu.org/msg43910.html

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |joseph at codesourcery dot
                   |                            |com, manu at gcc dot
                   |                            |gnu.org

--- Comment #1 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-07 20:42:23 UTC ---
Joseph, do we want to do this or not? We could always move -Wformat-zero-length
to -Wformat=2 or to -Wextra (or both)

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

--- Comment #2 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-04-23 19:58:15 UTC ---
There are plenty of warnings in -Wall that relate to things that are 
unusual but may be OK in some cases, or where -Wall expects a particular 
coding style to be used.  I think -Wformat-zero-length falls into the 
category of pointing out something that might be OK but is likely not to 
be intended.  So I think it's fine in -Wall - but if a subset of format 
checking warnings were enabled by default (as in Ubuntu, for example) then 
maybe that default subset would not include this one.

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

--- Comment #3 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-23 20:08:09 UTC ---
Well, I don't really see how custom_printf("") can produce any damage. It may
be an oversight when one actually wanted to print something, but it may be as
likely that one didn't want to print something and just trigger some side
effect of custom_printf.

As an alternative, the warning could suggest to use ("%s", "") to silence the
warning, no?

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[andersk at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

--- Comment #4 from Anders Kaseorg <andersk at mit dot edu> 2012-04-23 20:10:22 UTC ---
Yes, I understand what -Wall is supposed to mean.

I don’t have a problem with -Wall warning about ‘if (foo = 3)’ when I probably
intended ‘if (foo == 3)’ and I could have written ‘if ((foo = 3))’ if that’s
what I really wanted.

I don’t have a problem with -Wall warning about ‘printf("hello", "world")’ when
I probably intended ‘printf("hello %s", "world")’ and I could have written
‘printf("hello")’ if that’s what I really wanted.

But ‘custom_printf_like_function(foo, "")’ is something different.  What do you
think that line indicates I intended?  What coding style do you think -Wall is
expecting if I really meant the empty string?

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[andersk at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

--- Comment #5 from Anders Kaseorg <andersk at mit dot edu> 2012-04-23 20:22:20 UTC ---
I’m not sure ("%s", "") is a suitable replacement in general.  Maybe this is a
far-fetched example, but what the purpose of custom_printf is to shell-quote
all its arguments, so that custom_printf(foo, "") writes "some_command" with no
arguments but custom_printf(foo, "%s", "") writes "some_command ''" with a
single empty argument?

In any event, ("%s", "") is certainly different code with a potential runtime
cost, and it’s not fair for -Wall to complain about the more straightforward
("") unless it’s reasonably likely for that code to be hiding some class of
bugs.  Is it?

(Another real-world example from a project I help maintain:
https://github.com/barnowl/barnowl/blob/barnowl-1.8.1/keys.c#L337 )

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

--- Comment #6 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-04-23 20:49:38 UTC ---
-Wall is expecting printf-like functions where empty strings are useless 
as arguments and might indicate e.g. cruft you hadn't completely cleaned 
up from your program.  Or a style where you have separate specialized 
functions for such cases (like the style where you carefully use fputs, 
fputc etc. for cases that can be covered by those functions, instead of 
using printf universally).

There's nothing wrong with using a -Wno- option (or associated pragmas) if 
the stylistic choices for your program are different from those for -Wall.  
Wall tries to strike a reasonable balance, but won't be perfect for 
everyone; it's a good starting point for both enabling further warnings, 
and disabling other warnings, to taste.

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[andersk at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

--- Comment #7 from Anders Kaseorg <andersk at mit dot edu> 2012-04-23 21:31:18 UTC ---
That’s a _much_ higher-level style decision than assumed by any of the other
-Wall warnings (or indeed any other warning switches at all), and a
questionable one at that.  -Wall should not encourage me to duplicate all my
custom printf-like functions just so they can be used with the empty string.

I understand that I can turn this warning off, but this bug is about the
defaults, and defaults are important.

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

--- Comment #8 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-26 14:19:13 UTC ---
I still think it may be fine moving this warning to -Wextra, since it gives
false positives and when it does, they are hard to avoid.

Anders, you may get to convince more people by sending a patch to gcc-patches
with your reasoning.

[Bug c/47901] -Wall should not imply -Wformat-zero-length by default

[naesten at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901

Samuel Bronson <naesten at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |naesten at gmail dot com

--- Comment #9 from Samuel Bronson <naesten at gmail dot com> ---
Wouldn't the case where a given custom printf-like function does more than just
print the formatted text be better solved by adding a new __attribute__ to
disable this warning for such a function?  (Assuming anyone actually wants
these warnings ever ...)

Then, you could still have warnings for code like 'printf();' that does
nothing, while avoiding them for your 'custom_printf(foo, "");' that does do
something.