Re: [Qemu-devel] [PATCH 8/9] Add more format string warning flags

["Daniel P. Berrange" <berrange@redhat.com>]

On Mon, Apr 02, 2012 at 03:04:54PM +0100, Peter Maydell wrote:
> On 2 April 2012 13:17, Daniel P. Berrange <berrange@redhat.com> wrote:
> > On Mon, Apr 02, 2012 at 01:13:56PM +0100, Peter Maydell wrote:
> >> On 2 April 2012 11:50, Daniel P. Berrange <berrange@redhat.com> wrote:
> >> > +#if defined __GNUC__
> >> > +# define GCC_WARNINGS_SAVE      _Pragma("GCC diagnostic push")
> >> > +# define GCC_WARNINGS_RESTORE   _Pragma("GCC diagnostic pop")
> >> > +# define DO_PRAGMA(x)           _Pragma(#x)
> >> > +# define GCC_WARNINGS_IGNORE(x) DO_PRAGMA(GCC diagnostic ignored x)
> >> > +#else
> >> > +# define GCC_WARNINGS_SAVE
> >> > +# define GCC_WARNINGS_RESTORE
> >> > +# define GCC_WARNINGS_IGNORE(x)
> >> > +#endif
> >>
> >> Do these pragmas work on all versions of gcc that we support?
> >> Google suggests that the push/pop ones are only gcc 4.6 or better,
> >> for example.
> >
> > Hmm, the gcc info pages didn't mention any version constraints, but I'll
> > investigate this
> 
> Having thought about it a little more, to be honest I'm not really
> convinced that we should have these anyway. Better just to not enable
> the format-nonliteral warning. (format-security should catch the cases
> we care most about anyway, I think.)

The -Wformat-security option can only catch problems if the format
string is a literal. eg so it'd miss this:

  void foo(void) {
     int notastring = 1;
     const char *format = "String is %s";

     sprintf(format, notastring);
  }

There are a handful of places in QEMU which do that with non-trivial
format strings & were easy to fix in this patch, which I think is a
worthwhile improvement. The cases in the *-user/strace.c file though
are not practical to fix, without significant re-design of the code
in question.

I'm fine if we just include those easy fixes, while leaving the actual
warning flag disabled for now - someone can revisit later if desired.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|