ewx: (geek)
Richard Kettlewell ([personal profile] ewx) wrote2009-04-27 02:49 pm

Stupid GCC

$ cat t.c
#include <stdio.h>
int main(void) { return printf(""); }
$ gcc -Wall -c t.c
t.c: In function ‘main’:
t.c:2: warning: zero-length printf format string
$ gcc -Wall -Wno-format-zero-length -c t.c
$ gcc --version
gcc (Debian 4.3.2-1.1) 4.3.2
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Can anyone offer a plausible reason why:

  • -Wformat-zero-length is on by default (i.e. implied by -Wformat and thus by -Wall)?
  • Why it exists at all?

FTAOD, empty format strings are perfectly legitimate (and the GCC Manual knows this).

fanf: (Default)

[personal profile] fanf 2009-04-27 02:05 pm (UTC)(link)
I'm wondering in what circumstances it would be reasonable to have a zero-length format string, instead of just doing nothing.
ext_8103: (Default)

[identity profile] ewx.livejournal.com 2009-04-27 02:10 pm (UTC)(link)
You're calling a prompt-and-input function and in some circumstances you don't want to print one or you've printed one already.
fanf: (Default)

[personal profile] fanf 2009-04-27 02:14 pm (UTC)(link)

That suggests you're using a non-literal format string, which is a different code smell. But if you aren't, why are you writing:

    if (need_prompt)
        printf(">");
    else
        printf("");

instead of

    if (need_prompt)
        printf(">");

?

simont: A picture of me in 2016 (Default)

[personal profile] simont 2009-04-27 02:16 pm (UTC)(link)
I think [livejournal.com profile] ewx meant the case where you've defined a function along the lines of
char *read_from_terminal(char *promptfmt, ...);
and sometimes you want to read from terminal with a zero-length prompt, in which case the obvious idiom is read_from_terminal("") rather than going out of your way to have a second read_from_terminal_without_prompt() function.
ext_8103: (Default)

[identity profile] ewx.livejournal.com 2009-04-27 02:19 pm (UTC)(link)
That's exactly right.
simont: A picture of me in 2016 (Default)

[personal profile] simont 2009-04-27 02:21 pm (UTC)(link)
Workaround: read_from_terminal("%.*s", 0, "-Wformat-zero-length please choke on a bucket of socks");

eta: second comment on this entry I've had to edit to fix a misplaced negative. Perhaps I shouldn't have got out of bed.
Edited 2009-04-27 14:22 (UTC)
fanf: (Default)

[personal profile] fanf 2009-04-27 02:25 pm (UTC)(link)
Ah right, I suppose that's a reasonable scenario. Though read_from_terminal() could be a wrapper around v*printf() and a function that does the actual reading, which you could call instead.

[identity profile] imc.livejournal.com 2009-04-27 03:03 pm (UTC)(link)
That would presumably have to be a macro though, not a function, to generate the warning.
simont: A picture of me in 2016 (Default)

[personal profile] simont 2009-04-27 03:06 pm (UTC)(link)
No, because if you're defining a function like that then you certainly use gcc's __attribute__((printf)) feature.
simont: A picture of me in 2016 (Default)

[personal profile] simont 2009-04-27 02:13 pm (UTC)(link)
The obvious case is when your format strings are generated at compile time by a system of cpp macros, and some of them come out empty.

(I've often thought that a lot of compiler warnings need a special case "unless it was generated by a macro", because the sorts of things that indicate a probable user error when they appear in handwritten code are often very similar to the sorts of things that are perfectly sensible preprocessor output.)
Edited 2009-04-27 14:14 (UTC)
ext_8103: (Default)

[identity profile] ewx.livejournal.com 2009-04-27 02:24 pm (UTC)(link)
An "unless unreachable" clause in particular ought to be a no-brainer, and would have saved at least two false positives that I can remember.
simont: A picture of me in 2016 (Default)

[personal profile] simont 2009-04-27 02:33 pm (UTC)(link)
Or "unless I use the general-purpose warning-suppression escape". Things like the extra pair of parentheses around assignments in if statements are all very well, but really one would like an __I_meant_to_do_this(arbitrary C code) sort of construction which would work equally well against any warning, with which you could then liberally pepper your complicated macro setups.

(In a really ideal world it would also work equally well in any compiler, but the boat is well and truly missed on that one.)

[identity profile] geekette8.livejournal.com 2009-04-27 02:06 pm (UTC)(link)
There are plenty of things that are perfectly legitimate but still merit warnings, tho, so the fact that it's legitimate as per the spec is no guidance at all as to whether it is or should be a warning!

At a guess it's there in the first place because somebody did it by mistake, thought "That should be a warning, nobody is likely to need to do that" and so they put it in.

It doesn't seem unreasonable to me given

-Wall: This enables all the warnings about constructions that some users consider questionable, and that are easy to avoid (or modify to prevent the warning)[...]
fanf: (Default)

[personal profile] fanf 2009-04-27 02:11 pm (UTC)(link)
The comments in gcc/c-format.c say that zero-length format strings are similar to excess arguments following the format string, which is a bit of a stretch...

[identity profile] bellinghman.livejournal.com 2009-04-27 02:40 pm (UTC)(link)
Is it?

In both cases, something which might be expected to have no effect has been coded. Actually, given that excess arguments could be used for their side effects, there might be an argument that an empty format string is even less useful than excess arguments whose evaluation does something useful.

Also, given the same argument about format strings wrapped in macros, one could end up with a sequence like

printf("%s %d", StrArg1, IntArg1, IntArg2, IntArg3);
printf("%s %d %d", StrArg1, IntArg1, IntArg2, IntArg3);
printf("%s: (%d, %d)", StrArg1, IntArg1, IntArg2, IntArg3);

etc. being produced, and being warned about becuase not all the arguments get used.
ext_8103: (Default)

something which might be expected to have no effect has been coded

[identity profile] ewx.livejournal.com 2009-04-27 03:49 pm (UTC)(link)
...well, yes, but in the case of printf("") it does have no effect. That's rather different from the general excess-arguments case!

[identity profile] geekette8.livejournal.com 2009-04-27 02:11 pm (UTC)(link)
In fact, the more I think about it, the more I can't see why it shouldn't be a valid warning!

Would you say
if(foo);
{ bar(); }
should not be warned about, just because it's perfectly legitimate C code?

Why would you want to do a zero-length format string deliberately anyway?
ext_8103: (Default)

[identity profile] ewx.livejournal.com 2009-04-27 02:14 pm (UTC)(link)

Usually the things that merit warnings are those for which the compiler won't do what the user might reasonably have expected. if(a=b) generates a warning because the programmer probably might plausibly have expected comparison rather than assignment, for instance.

An empty format string will do exactly what the user expected, though. You don't enter "" expecting to get "Hello, world!" out.

[identity profile] pjc50.livejournal.com 2009-04-27 03:19 pm (UTC)(link)
My pet complaint is the introduction of more aggressive -Wparen in 4.3; it now warns about if(a && b || c). I suppose there's something to be said for it, but nobody wants to go and retroactively change a million lines of C++ for it..
simont: A picture of me in 2016 (Default)

[personal profile] simont 2009-04-27 03:33 pm (UTC)(link)
That's new in 4.3?! I'm sure I remember "suggest parentheses around && within ||" from many years ago.

When I'm designing languages these days I tend to make "a && b || c" actually illegal: && and || have the same priority but will each only associate with themselves.

[identity profile] cartesiandaemon.livejournal.com 2009-04-28 09:51 am (UTC)(link)
Yeah, I can really see that. Or maybe the operators should be "&" an "||||" or something, so it LOOKS like what it does :)
ext_8103: (Default)

[identity profile] ewx.livejournal.com 2009-04-27 03:34 pm (UTC)(link)

I assume you mean -Wparentheses - if not then I must have some other GCC 4.3 to you! However, that construction has generated a warning for quite some time now:

$ cat t.c
int foo(int a, int b, int c) { return a && b || c; }
$ gcc -Wparentheses -c t.c
t.c: In function `foo':
t.c:1: warning: suggest parentheses around && within ||
$ gcc --version
2.95.2

[identity profile] pjc50.livejournal.com 2009-04-27 04:17 pm (UTC)(link)
Yes, I meant -Wparentheses. In fact we're both right - using your t.c as "wall.c":

$ gcc -Wall -c wall.c
wall.c: In function ‘foo’:
wall.c:1: warning: suggest parentheses around && within ||
$ g++ -Wall -c wall.c
$

... well, that's helpful, isn't it.
G++ 4.3 and above appear to be much better about warnings in C++ code. Even our old version of boost from ~3 years ago is not accepted.
ext_8103: (Default)

[identity profile] ewx.livejournal.com 2009-04-27 04:28 pm (UTC)(link)
Ha. I refer you to the title at the top of this page l-)

[identity profile] fluffymormegil.livejournal.com 2009-04-27 05:18 pm (UTC)(link)
g++ 4.3's -Wunreachable-code is worthless, because it generates spurious gerbil spew for its own STL.
ext_8103: (Default)

[identity profile] ewx.livejournal.com 2009-04-27 06:30 pm (UTC)(link)
-isystem is supposed to deal with that sort of thing...
pm215: (Default)

[personal profile] pm215 2009-04-27 07:57 pm (UTC)(link)
The gcc mailing list post giving the original patch to add -Wformat-zero-length explains why it's there: this used to be a standard part of -Wformat and the patch submitter wanted to be able to turn it off without turning off all of -Wformat (for exactly the kind of reasons you give :-)). It's on by default because they didn't want to change the default behaviour from gcc 2.95:
mnementh$ gcc-2.95 -Wall -c t.c
t.c: In function `main':
t.c:2: warning: zero-length format string
mnementh$ gcc-2.95 --version
2.95.4

and also because "There are lots of other checks for useless formats (e.g., for using useless combinations of flag characters where one gets ignored)".

pm215: (Default)

[personal profile] pm215 2012-03-13 02:22 pm (UTC)(link)
...since this came up on irc again today, I looked through some older gcc releases. "zero-length format string" has been warned about since gcc 2.0 (gcc 1.42 has no format string checking at all). Whenever this came in it seems to be before the dawn of gcc's online svn repo history.

GCC bug

[identity profile] andersk.livejournal.com 2012-04-04 05:04 pm (UTC)(link)
You should all add your thoughts to GCC bug 47901 “-Wall should not imply -Wformat-zero-length by default” (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47901).