Re: [Qemu-devel] [PATCH 1/9] build: disable Wstrict-prototypes

[Kevin Wolf]

On Wed, Feb 20, 2013 at 04:33:50PM +0100, Laszlo Ersek wrote:
> On 02/20/13 15:57, Kevin Wolf wrote:
> > On Wed, Feb 20, 2013 at 03:50:11PM +0100, Laszlo Ersek wrote:
> >> On 02/20/13 12:31, Kevin Wolf wrote:
> >>> On Mon, Feb 18, 2013 at 05:56:57PM -0600, Anthony Liguori wrote:
> >>>> GTK won't build with strict-prototypes due to gtkitemfactory.h:
> >>>>
> >>>>     /* We use () here to mean unspecified arguments. This is deprecated
> >>>>      * as of C99, but we can't change it without breaking compatibility.
> >>>>      * (Note that if we are included from a C++ program () will mean
> >>>>      * (void) so an explicit cast will be needed.)
> >>>>      */
> >>>>     typedef      void    (*GtkItemFactoryCallback)  ();
> >>>>
> >>>> Signed-off-by: Anthony Liguori <address@hidden>
> >>>> ---
> >>>>  configure | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/configure b/configure
> >>>> index bf5970f..74d5878 100755
> >>>> --- a/configure
> >>>> +++ b/configure
> >>>> @@ -283,7 +283,7 @@ sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
> >>>>  # default flags for all hosts
> >>>>  QEMU_CFLAGS="-fno-strict-aliasing $QEMU_CFLAGS"
> >>>>  QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
> >>>> $QEMU_CFLAGS"
> >>>> -QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
> >>>> +QEMU_CFLAGS="-Wredundant-decls $QEMU_CFLAGS"
> >>>>  QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
> >>>> $QEMU_CFLAGS"
> >>>>  QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/include"
> >>>>  if test "$debug_info" = "yes"; then
> >>>
> >>> Other places wrap the inclusion of problematic headers in '#pragma GCC
> >>> diagnostic ...' instead of globally disabling warnings.
> >>
> >> Available only in gcc-4.6+:
> >>
> >> http://gcc.gnu.org/gcc-4.6/changes.html
> > 
> > I think this is mostly the push/pop part. We avoid it for this reason
> > and just unconditionally make it error after including the header file.
> > The pragma is also only enabled by a configure check.
> > 
> > Have a look at include/ui/qemu-pixman.h or coroutine-ucontext.c for an
> > example.
> 
> Yes, I checked those before posting. Two points:
> 
> (a) #pragma PP_TOKEN1 PP_TOKEN2 ..., where PP_TOKEN1 is not STDC, has
> implementation-defined effect. (Ie. the platform can do whatever it
> wants, but has to document it). Non-STDC #pragmas that are not
> recognized by the implementation must be ignored (C99 6.10.6p1
> requirement). So wrapping "#pragma GCC diagnostic" is not really useful:
> non-gcc compilers will surely not recognize "#pragma GCC whatever", and
> an older gcc (not recognizing "#pragma GCC diagnostic") is required to
> ignore it.

gcc produces a warning for unknown pragmas, and -Werror turns it into an
error, so it's not quite useless.

> (b) My main point was, if you build on a gcc which understands
> -Wstrict-prototypes and chokes on gtkitemfactory.h, but doesn't
> understand the #pragma, then compilation will fail.
> 
> I think that on such a platform "qemu-pixman.h" / <pixman.h> from
> pixman-0.16.0 would fail to parse as well.

I think you're right here. When I originally introduced the first one of
these, I had to deal with a new warning in 4.6, so this was obviously
not a problem. But qemu-pixman.h as well as this one can't rely on it.

Maybe we need to remove -Werror if configure detects that the #pragma
doesn't work.

Kevin