[libvirt] PATCH: Fix compiler flag check for -Wformat-security
Certain versions of GCC will ignore the -Wformat-security flag unless you have also set the -Wformat flag. Unfortunately when checking flags, we only check one at a time in isolation, so it was thinking -Wformat-security is not supported in Fedora. The fix is to include all previously verified flags during the check process, but not for -Werror because for added fun, the autoconf test program itself causes compile warnings with some flags we have. The patch also fixes 3 harmless format problems in virsh Daniel Index: m4/compiler-flags.m4 =================================================================== RCS file: /data/cvs/libvirt/m4/compiler-flags.m4,v retrieving revision 1.2 diff -u -p -r1.2 compiler-flags.m4 --- m4/compiler-flags.m4 14 Jan 2009 15:23:26 -0000 1.2 +++ m4/compiler-flags.m4 27 Apr 2009 10:34:43 -0000 @@ -24,7 +24,15 @@ AC_DEFUN([gl_COMPILER_FLAGS], [AC_MSG_CHECKING(whether compiler accepts $1) AC_SUBST(COMPILER_FLAGS) ac_save_CFLAGS="$CFLAGS" - CFLAGS="$CFLAGS $1" + dnl Some flags are dependant, so we set all previously checked + dnl flags when testing. Except for -Werror which we have to + dnl check on its own, because some of our compiler flags cause + dnl warnings from the autoconf test program! + if test "$1" = "-Werror" ; then + CFLAGS="$CFLAGS $1" + else + CFLAGS="$CFLAGS $COMPILER_FLAGS $1" + fi AC_TRY_LINK([], [], has_option=yes, has_option=no,) echo 'int x;' >conftest.c $CC $CFLAGS -c conftest.c 2>conftest.err Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.201 diff -u -p -r1.201 virsh.c --- src/virsh.c 15 Apr 2009 20:09:09 -0000 1.201 +++ src/virsh.c 27 Apr 2009 10:34:45 -0000 @@ -4463,7 +4463,7 @@ cmdNodeListDevicesPrint(vshControl *ctl, } /* Print this device */ - vshPrint(ctl, indentBuf); + vshPrint(ctl, "%s", indentBuf); vshPrint(ctl, "%s\n", devices[devid]); @@ -4487,7 +4487,7 @@ cmdNodeListDevicesPrint(vshControl *ctl, /* If there is a child device, then print another blank line */ if (nextlastdev != -1) { - vshPrint(ctl, indentBuf); + vshPrint(ctl, "%s", indentBuf); vshPrint(ctl, " |\n"); } @@ -4511,7 +4511,7 @@ cmdNodeListDevicesPrint(vshControl *ctl, /* If there was no child device, and we're the last in * a list of devices, then print another blank line */ if (nextlastdev == -1 && devid == lastdev) { - vshPrint(ctl, indentBuf); + vshPrint(ctl, "%s", indentBuf); vshPrint(ctl, "\n"); } } -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, Apr 27, 2009 at 11:37:35AM +0100, Daniel P. Berrange wrote:
Certain versions of GCC will ignore the -Wformat-security flag unless you have also set the -Wformat flag. Unfortunately when checking flags, we only check one at a time in isolation, so it was thinking -Wformat-security is not supported in Fedora. The fix is to include all previously verified flags during the check process, but not for -Werror because for added fun, the autoconf test program itself causes compile warnings with some flags we have. The patch also fixes 3 harmless format problems in virsh
Thanks for chasing this issue, all looks good, ACK. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Apr 27, 2009 at 02:25:16PM +0200, Daniel Veillard wrote:
On Mon, Apr 27, 2009 at 11:37:35AM +0100, Daniel P. Berrange wrote:
Certain versions of GCC will ignore the -Wformat-security flag unless you have also set the -Wformat flag. Unfortunately when checking flags, we only
Hum, I noticed recently that some packages change the make output to just print something like the following when compiling a C module: CC module.c instead of the now 3-4 lines of compiler flags, which make spotting warnings way easier. Maybe this would be a good idea for us too, opinion ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Apr 27, 2009 at 02:28:58PM +0200, Daniel Veillard wrote:
On Mon, Apr 27, 2009 at 02:25:16PM +0200, Daniel Veillard wrote:
On Mon, Apr 27, 2009 at 11:37:35AM +0100, Daniel P. Berrange wrote:
Certain versions of GCC will ignore the -Wformat-security flag unless you have also set the -Wformat flag. Unfortunately when checking flags, we only
Hum, I noticed recently that some packages change the make output to just print something like the following when compiling a C module: CC module.c instead of the now 3-4 lines of compiler flags, which make spotting warnings way easier. Maybe this would be a good idea for us too, opinion ?
This is a little tricky todo with libtool, though you could hack up some semi-evil solution eg, you can hack the makefile.am to add --silent to libtool, but make itself will still print out the name of the command run (which is huge). So you also have to add -s, but then you get no output at all. So you have to add a wrapper script around libtool to run with --silent and also print the filename. Personally I always build with --enable-compile-warnings=error so the build stops immediately if any warning message is generated. That way you don't have to pay close attention to the messages scrolling up Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 04/27/2009 08:28 AM, Daniel Veillard wrote:
Hum, I noticed recently that some packages change the make output to just print something like the following when compiling a C module: CC module.c instead of the now 3-4 lines of compiler flags, which make spotting warnings way easier. Maybe this would be a good idea for us too, opinion ?
I prefer it that way too, as long as the flags are printed once for each final target (assuming the same cc flags are used for all source files used by that target). I wish I could unclutter my desk that easily!
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Laine Stump