On 10/12, Jonathan Nieder wrote:
> Jeff King wrote:
> > On Fri, Oct 12, 2018 at 07:40:37PM +0100, Thomas Gummerer wrote:
>
> >> 801fa63a90 ("config.mak.dev: add -Wformat-security", 2018-09-08) added
> >> the -Wformat-security to the flags set in config.mak.dev. In the gcc
> >> man page this is documented as:
> >>
> >> If -Wformat is specified, also warn about uses of format
> >> functions that represent possible security problems. [...]
> >>
> >> That commit did however not add the -Wformat flag, and -Wformat is not
> >> specified anywhere else by default, so the added -Wformat-security had
> >> no effect. Newer versions of gcc (gcc 8.2.1 in this particular case)
> >> warn about this and thus compilation fails with this option set.
> [...]
> > -Wformat is part of -Wall, which we already turn on by default (even for
> > non-developer builds).
> >
> > So I don't think we need to do anything more, though I'm puzzled that
> > you saw a failure. Do you set CFLAGS explicitly in your config.mak to
> > something that doesn't include -Wall?
Whoops embarrassing. I had this set in my config.mak:
CFLAGS = -O$(O) -g $(EXTRA_CFLAGS)
What happened is that I had included -Wall in an old config.mak that I
copied from Thomas Rast when I started with my GSoC project. Then
when "DEVELOPER=1" came around I switched to that at some point and
just removed everything from CFLAGS, except the possibility to
override the optimization level, the ability to add extra flags and
including debug symbols, but failed to notice that I had lost -Wall.
Maybe it would still be a good to add -Wall to avoid the surprise for
others. But then again if someone overrides CFLAGS they should at
least check better what they're overriding ;)
> Thomas, do you use autoconf to generate config.mak.autogen? I'm
> wondering if that produces a CFLAGS that doesn't include -Wall.
No, this was all my mistake :)
> > I'm not opposed to making config.mak.dev a bit more redundant to handle
> > this case, but we'd probably want to include all of -Wall, since it
> > contains many other warnings we'd want to make sure are enabled.
>
> Do you mean putting -Wall instead of -Wformat?
>
> Should we add -Wextra too? From a quick test, it seems to build okay.
We do have that with setting DEVELOPER=extra-all.
> Thanks,
> Jonathan
