[libvirt] [PATCH 0/6] Switch to use gnulib's manywarnings
A second version of http://www.redhat.com/archives/libvir-list/2011-April/msg00060.html In this version: - Move acinclude.m4 into m4/ - Don't blacklist -W, only use -Wno-sign-compare - Fix some issues allowing further warnings to be turned on - Increase max stack size to 65700 bytes to make test suite compile. Test suite to be fixed later :-)
Remove custom code for checking compiler warnings, using gl_WARN_ADD instead. Don't list all flags ourselves, use gnulib's gl_MANYWARN_ALL_GCC to get all possible GCC flags, then turn off the ones we don't want yet. * acinclude.m4: Rewrite to use gl_WARN_ADD and gl_MANYWARN_ALL_GCC * bootstrap.conf: Add warnings & manywarnings * configure.ac: Switch to gl_WARN_ADD * m4/compiler-flags.m4: Obsoleted by gl_WARN_ADD --- acinclude.m4 | 158 +++++++++++++++++++++++++++----------------------- bootstrap.conf | 2 + configure.ac | 15 +++-- m4/compiler-flags.m4 | 48 --------------- 4 files changed, 96 insertions(+), 127 deletions(-) delete mode 100644 m4/compiler-flags.m4 diff --git a/acinclude.m4 b/acinclude.m4 index 838ec46..8eb0ec5 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -1,11 +1,6 @@ dnl -dnl Taken from gnome-common/macros2/gnome-compiler-flags.m4 -dnl -dnl We've added: -dnl -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline -Wredundant-decls -dnl We've removed -dnl CFLAGS="$realsave_CFLAGS" -dnl to avoid clobbering user-specified CFLAGS +dnl Enable all known GCC compiler warnings, except for those +dnl we can't yet cope with dnl AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl ****************************** @@ -13,90 +8,107 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl ****************************** AC_ARG_ENABLE(compile-warnings, - [AC_HELP_STRING([--enable-compile-warnings=@<:@no/minimum/yes/maximum/error@:>@], + [AC_HELP_STRING([--enable-compile-warnings=@<:@no/yes/error@:>@], [Turn on compiler warnings])],, - [enable_compile_warnings="m4_default([$1],[maximum])"]) - - warnCFLAGS= - - common_flags= - common_flags="$common_flags -Wp,-D_FORTIFY_SOURCE=2" - common_flags="$common_flags -fexceptions" - common_flags="$common_flags -fasynchronous-unwind-tables" - common_flags="$common_flags -fdiagnostics-show-option" + [enable_compile_warnings="m4_default([$1],[yes])"]) case "$enable_compile_warnings" in no) try_compiler_flags="" ;; - minimum) - try_compiler_flags="-Wall -Wformat -Wformat-security $common_flags" - ;; - yes) - try_compiler_flags="-Wall -Wformat -Wformat-security -Wmissing-prototypes $common_flags" - ;; - maximum|error) - try_compiler_flags="-Wall -Wformat -Wformat-security" - try_compiler_flags="$try_compiler_flags -Wmissing-prototypes" - try_compiler_flags="$try_compiler_flags -Wnested-externs " - try_compiler_flags="$try_compiler_flags -Wpointer-arith" - try_compiler_flags="$try_compiler_flags -Wextra -Wshadow" - try_compiler_flags="$try_compiler_flags -Wcast-align" - try_compiler_flags="$try_compiler_flags -Wwrite-strings" - try_compiler_flags="$try_compiler_flags -Waggregate-return" - try_compiler_flags="$try_compiler_flags -Wstrict-prototypes" - try_compiler_flags="$try_compiler_flags -Winline" - try_compiler_flags="$try_compiler_flags -Wredundant-decls" - try_compiler_flags="$try_compiler_flags -Wno-sign-compare" - try_compiler_flags="$try_compiler_flags -Wlogical-op" - try_compiler_flags="$try_compiler_flags $common_flags" - if test "$enable_compile_warnings" = "error" ; then - try_compiler_flags="$try_compiler_flags -Werror" - fi + yes|minimum|maximum|error) + + # List of warnings that are not relevant / wanted + dontwarn="$dontwarn -Wc++-compat" # Don't care about C++ compiler compat + dontwarn="$dontwarn -Wtraditional" # Don't care about ancient C standard compat + dontwarn="$dontwarn -Wtraditional-conversion" # Don't care about ancient C standard compat + dontwarn="$dontwarn -Wsystem-headers" # Ignore warnings in /usr/include + dontwarn="$dontwarn -Wpadded" # Happy for compiler to add struct padding + dontwarn="$dontwarn -Wunreachable-code" # GCC very confused with -O2 + dontwarn="$dontwarn -Wconversion" # Too many to deal with + dontwarn="$dontwarn -Wsign-conversion" # Too many to deal with + dontwarn="$dontwarn -Wvla" # GNULIB gettext.h violates + dontwarn="$dontwarn -Wundef" # Many GNULIB violations + dontwarn="$dontwarn -Wcast-qual" # Need to allow bad cast for execve() + dontwarn="$dontwarn -Wlong-long" # We need to use long long in many places + dontwarn="$dontwarn -Wswitch-default" # We allow manual list of all enum cases without default: + dontwarn="$dontwarn -Wswitch-enum" # We allow optional default: instead of listing all enum values + dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -fstrict-overflow + dontwarn="$dontwarn -Wunsafe-loop-optimizations" # Not a problem since we don't use -funsafe-loop-optimizations + + # We might fundamentally need some of these disabled forever, but ideally + # we'd turn many of them on + dontwarn="$dontwarn -Wformat-nonliteral" + dontwarn="$dontwarn -Wfloat-equal" + dontwarn="$dontwarn -Wdeclaration-after-statement" + dontwarn="$dontwarn -Wcast-qual" + dontwarn="$dontwarn -Wconversion" + dontwarn="$dontwarn -Wsign-conversion" + dontwarn="$dontwarn -Wold-style-definition" + dontwarn="$dontwarn -Wmissing-noreturn" + dontwarn="$dontwarn -Wpacked" + dontwarn="$dontwarn -Wunused-macros" + dontwarn="$dontwarn -Woverlength-strings" + dontwarn="$dontwarn -Wmissing-format-attribute" + dontwarn="$dontwarn -Wstack-protector" + + # Get all possible GCC warnings + gl_MANYWARN_ALL_GCC([maybewarn]) + + # Remove the ones we don't want, blacklisted earlier + gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn]) + + # Check for $CC support of each warning + for w in $wantwarn; do + gl_WARN_ADD([$w]) + done + + # GNULIB uses '-W' which includes a bunch of stuff, + # kinda like -Wextra. Unfortunately, it means you + # can't simply use '-Wsign-compare' with gl_MANYWARN_COMPLEMENT + # So we have -W enabled, and then have to explicitly turn off + gl_WARN_ADD(-Wno-sign-compare) + + # This should be < 256 really, but with PATH_MAX everywhere + # we have doom, even with 4096. In fact we have some functions + # with several PATH_MAX sized variables :-( We should kill off + # all PATH_MAX usage and then lower this limit + gl_WARN_ADD([-Wframe-larger-than=65700]) + dnl gl_WARN_ADD([-Wframe-larger-than=4096]) + dnl gl_WARN_ADD([-Wframe-larger-than=256]) + + # Extra special flags + gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) + dnl Fedora only uses -fstack-protector, but doesn't seem to + dnl be great overhead in adding -fstack-protector-all instead + dnl gl_WARN_ADD([-fstack-protector]) + gl_WARN_ADD([-fstack-protector-all]) + gl_WARN_ADD([--param=ssp-buffer-size=4]) + gl_WARN_ADD([-fexceptions]) + gl_WARN_ADD([-fasynchronous-unwind-tables]) + gl_WARN_ADD([-fdiagnostics-show-option]) + + if test "$enable_compile_warnings" = "error" + then + gl_WARN_ADD([-Werror]) + fi ;; *) AC_MSG_ERROR(Unknown argument '$enable_compile_warnings' to --enable-compile-warnings) ;; esac - COMPILER_FLAGS= - for option in $try_compiler_flags; do - gl_COMPILER_FLAGS($option) - done - unset option - unset try_compiler_flags - - AC_ARG_ENABLE(iso-c, - AC_HELP_STRING([--enable-iso-c], - [Try to warn if code is not ISO C ]),, - [enable_iso_c=no]) - - AC_MSG_CHECKING(what language compliance flags to pass to the C compiler) - complCFLAGS= - if test "x$enable_iso_c" != "xno"; then - if test "x$GCC" = "xyes"; then - case " $CFLAGS " in - *[\ \ ]-ansi[\ \ ]*) ;; - *) complCFLAGS="$complCFLAGS -ansi" ;; - esac - case " $CFLAGS " in - *[\ \ ]-pedantic[\ \ ]*) ;; - *) complCFLAGS="$complCFLAGS -pedantic" ;; - esac - fi - fi - AC_MSG_RESULT($complCFLAGS) - - WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" WARN_LDFLAGS=$WARN_CFLAGS AC_SUBST([WARN_CFLAGS]) AC_SUBST([WARN_LDFLAGS]) dnl Needed to keep compile quiet on python 2.4 - COMPILER_FLAGS= - gl_COMPILER_FLAGS(-Wno-redundant-decls) - WARN_PYTHON_CFLAGS=$COMPILER_FLAGS + save_WARN_CFLAGS=$WARN_CFLAGS + WARN_CFLAGS= + gl_WARN_ADD(-Wno-redundant-decls) + WARN_PYTHON_CFLAGS=$WARN_CFLAGS AC_SUBST(WARN_PYTHON_CFLAGS) + WARN_CFLAGS=$save_WARN_CFLAGS ]) diff --git a/bootstrap.conf b/bootstrap.conf index ca0c3de..86aad14 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -45,6 +45,7 @@ ignore-value inet_pton ioctl maintainer-makefile +manywarnings mkstemp mkstemps mktempd @@ -81,6 +82,7 @@ timegm uname useless-if-before-free usleep +warnings vasprintf verify vc-list-files diff --git a/configure.ac b/configure.ac index a8d223a..7874c30 100644 --- a/configure.ac +++ b/configure.ac @@ -1970,12 +1970,14 @@ AC_ARG_ENABLE([test-coverage], enable_coverage=$enableval if test "${enable_coverage}" = yes; then - COMPILER_FLAGS= - gl_COMPILER_FLAGS(-fprofile-arcs) - gl_COMPILER_FLAGS(-ftest-coverage) - AC_SUBST([COVERAGE_CFLAGS], [$COMPILER_FLAGS]) - AC_SUBST([COVERAGE_LDFLAGS], [$COMPILER_FLAGS]) - COMPILER_FLAGS= + save_WARN_CFLAGS=$WARN_CFLAGS + WARN_CFLAGS= + gl_WARN_ADD(-fprofile-arcs) + gl_WARN_ADD(-ftest-coverage) + COVERAGE_FLAGS=$WARN_CFLAGS + AC_SUBST([COVERAGE_CFLAGS], [$COVERAGE_FLAGS]) + AC_SUBST([COVERAGE_LDFLAGS], [$COVERAGE_FLAGS]) + WARN_CFLAGS=$save_WARN_CFLAGS fi AC_ARG_ENABLE([test-oom], @@ -2538,6 +2540,7 @@ AC_MSG_NOTICE([Miscellaneous]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ Debug: $enable_debug]) AC_MSG_NOTICE([ Warnings: $enable_compile_warnings]) +AC_MSG_NOTICE([Warning Flags: $WARN_CFLAGS]) AC_MSG_NOTICE([ Readline: $lv_use_readline]) AC_MSG_NOTICE([ Python: $with_python]) AC_MSG_NOTICE([ DTrace: $with_dtrace]) diff --git a/m4/compiler-flags.m4 b/m4/compiler-flags.m4 deleted file mode 100644 index 6db4816..0000000 --- a/m4/compiler-flags.m4 +++ /dev/null @@ -1,48 +0,0 @@ -# serial 4 -# Find valid warning flags for the C Compiler. -*-Autoconf-*- -# -# Copyright (C) 2010 Red Hat, Inc. -# Copyright (C) 2001, 2002, 2006 Free Software Foundation, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA -# 02110-1301 USA - -# Written by Jesse Thilo. - -AC_DEFUN([gl_COMPILER_FLAGS], - [AC_MSG_CHECKING(whether compiler accepts $1) - ac_save_CFLAGS="$CFLAGS" - 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 - ret=$? - if test $ret != 0 || test -s conftest.err || test $has_option = "no"; then - AC_MSG_RESULT(no) - else - AC_MSG_RESULT(yes) - COMPILER_FLAGS="$COMPILER_FLAGS $1" - fi - CFLAGS="$ac_save_CFLAGS" - rm -f conftest* - ]) -- 1.7.4
On 04/04/2011 10:19 AM, Daniel P. Berrange wrote:
Remove custom code for checking compiler warnings, using gl_WARN_ADD instead. Don't list all flags ourselves, use gnulib's gl_MANYWARN_ALL_GCC to get all possible GCC flags, then turn off the ones we don't want yet.
* acinclude.m4: Rewrite to use gl_WARN_ADD and gl_MANYWARN_ALL_GCC * bootstrap.conf: Add warnings & manywarnings * configure.ac: Switch to gl_WARN_ADD * m4/compiler-flags.m4: Obsoleted by gl_WARN_ADD --- acinclude.m4 | 158 +++++++++++++++++++++++++++----------------------- bootstrap.conf | 2 + configure.ac | 15 +++-- m4/compiler-flags.m4 | 48 --------------- 4 files changed, 96 insertions(+), 127 deletions(-) delete mode 100644 m4/compiler-flags.m4
+ + # List of warnings that are not relevant / wanted + dontwarn="$dontwarn -Wc++-compat" # Don't care about C++ compiler compat + dontwarn="$dontwarn -Wtraditional" # Don't care about ancient C standard compat + dontwarn="$dontwarn -Wtraditional-conversion" # Don't care about ancient C standard compat + dontwarn="$dontwarn -Wsystem-headers" # Ignore warnings in /usr/include + dontwarn="$dontwarn -Wpadded" # Happy for compiler to add struct padding + dontwarn="$dontwarn -Wunreachable-code" # GCC very confused with -O2 + dontwarn="$dontwarn -Wconversion" # Too many to deal with + dontwarn="$dontwarn -Wsign-conversion" # Too many to deal with + dontwarn="$dontwarn -Wvla" # GNULIB gettext.h violates + dontwarn="$dontwarn -Wundef" # Many GNULIB violations + dontwarn="$dontwarn -Wcast-qual" # Need to allow bad cast for execve() + dontwarn="$dontwarn -Wlong-long" # We need to use long long in many places + dontwarn="$dontwarn -Wswitch-default" # We allow manual list of all enum cases without default: + dontwarn="$dontwarn -Wswitch-enum" # We allow optional default: instead of listing all enum values + dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -fstrict-overflow + dontwarn="$dontwarn -Wunsafe-loop-optimizations" # Not a problem since we don't use -funsafe-loop-optimizations
Seems okay (but long lines - any way to break that into 80 columns?)
+ + # We might fundamentally need some of these disabled forever, but ideally + # we'd turn many of them on + dontwarn="$dontwarn -Wformat-nonliteral"
This one may need to always be disabled, thanks to virAsprintf (it would be nicer if we could disable for just one or two files, rather than globally).
+ dontwarn="$dontwarn -Wfloat-equal" + dontwarn="$dontwarn -Wdeclaration-after-statement"
If gcc would do better, I'd love to guarantee C99 and take advantage of this (smaller scopes have maintenance benefits) - thus, I envision this one being permanent (float it up to the earlier set).
+ dontwarn="$dontwarn -Wcast-qual" + dontwarn="$dontwarn -Wconversion" + dontwarn="$dontwarn -Wsign-conversion" + dontwarn="$dontwarn -Wold-style-definition" + dontwarn="$dontwarn -Wmissing-noreturn" + dontwarn="$dontwarn -Wpacked" + dontwarn="$dontwarn -Wunused-macros" + dontwarn="$dontwarn -Woverlength-strings" + dontwarn="$dontwarn -Wmissing-format-attribute" + dontwarn="$dontwarn -Wstack-protector"
Yes, I agree that the rest of these should be temporary, and are probably worth fixing.
+ + # Get all possible GCC warnings + gl_MANYWARN_ALL_GCC([maybewarn]) + + # Remove the ones we don't want, blacklisted earlier + gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn]) + + # Check for $CC support of each warning + for w in $wantwarn; do + gl_WARN_ADD([$w]) + done + + # GNULIB uses '-W' which includes a bunch of stuff, + # kinda like -Wextra. Unfortunately, it means you
"kinda like"? Exactly like! -W was the old spelling, -Wextra was the new one (added quite some time ago; something like gcc 3.4, off the top of my head).
+ # can't simply use '-Wsign-compare' with gl_MANYWARN_COMPLEMENT + # So we have -W enabled, and then have to explicitly turn off + gl_WARN_ADD(-Wno-sign-compare) + + # This should be < 256 really, but with PATH_MAX everywhere + # we have doom, even with 4096. In fact we have some functions + # with several PATH_MAX sized variables :-( We should kill off + # all PATH_MAX usage and then lower this limit + gl_WARN_ADD([-Wframe-larger-than=65700]) + dnl gl_WARN_ADD([-Wframe-larger-than=4096]) + dnl gl_WARN_ADD([-Wframe-larger-than=256]) + + # Extra special flags + gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) + dnl Fedora only uses -fstack-protector, but doesn't seem to + dnl be great overhead in adding -fstack-protector-all instead + dnl gl_WARN_ADD([-fstack-protector]) + gl_WARN_ADD([-fstack-protector-all]) + gl_WARN_ADD([--param=ssp-buffer-size=4]) + gl_WARN_ADD([-fexceptions]) + gl_WARN_ADD([-fasynchronous-unwind-tables]) + gl_WARN_ADD([-fdiagnostics-show-option]) + + if test "$enable_compile_warnings" = "error" + then + gl_WARN_ADD([-Werror]) + fi
Looks like a good conversion.
@@ -81,6 +82,7 @@ timegm uname useless-if-before-free usleep +warnings vasprintf verify vc-list-files
Where did you learn to sort? :)
diff --git a/configure.ac b/configure.ac index a8d223a..7874c30 100644 --- a/configure.ac +++ b/configure.ac @@ -1970,12 +1970,14 @@ AC_ARG_ENABLE([test-coverage], enable_coverage=$enableval
if test "${enable_coverage}" = yes; then - COMPILER_FLAGS= - gl_COMPILER_FLAGS(-fprofile-arcs) - gl_COMPILER_FLAGS(-ftest-coverage) - AC_SUBST([COVERAGE_CFLAGS], [$COMPILER_FLAGS]) - AC_SUBST([COVERAGE_LDFLAGS], [$COMPILER_FLAGS]) - COMPILER_FLAGS= + save_WARN_CFLAGS=$WARN_CFLAGS + WARN_CFLAGS= + gl_WARN_ADD(-fprofile-arcs) + gl_WARN_ADD(-ftest-coverage)
Nit: use consistent m4 quoting: gl_WARN_ADD([-fprofile-arcs]) ACK with nits addressed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Apr 04, 2011 at 02:29:06PM -0600, Eric Blake wrote:
On 04/04/2011 10:19 AM, Daniel P. Berrange wrote:
Remove custom code for checking compiler warnings, using gl_WARN_ADD instead. Don't list all flags ourselves, use gnulib's gl_MANYWARN_ALL_GCC to get all possible GCC flags, then turn off the ones we don't want yet.
* acinclude.m4: Rewrite to use gl_WARN_ADD and gl_MANYWARN_ALL_GCC * bootstrap.conf: Add warnings & manywarnings * configure.ac: Switch to gl_WARN_ADD * m4/compiler-flags.m4: Obsoleted by gl_WARN_ADD --- acinclude.m4 | 158 +++++++++++++++++++++++++++----------------------- bootstrap.conf | 2 + configure.ac | 15 +++-- m4/compiler-flags.m4 | 48 --------------- 4 files changed, 96 insertions(+), 127 deletions(-) delete mode 100644 m4/compiler-flags.m4
+ + # List of warnings that are not relevant / wanted + dontwarn="$dontwarn -Wc++-compat" # Don't care about C++ compiler compat + dontwarn="$dontwarn -Wtraditional" # Don't care about ancient C standard compat + dontwarn="$dontwarn -Wtraditional-conversion" # Don't care about ancient C standard compat + dontwarn="$dontwarn -Wsystem-headers" # Ignore warnings in /usr/include + dontwarn="$dontwarn -Wpadded" # Happy for compiler to add struct padding + dontwarn="$dontwarn -Wunreachable-code" # GCC very confused with -O2 + dontwarn="$dontwarn -Wconversion" # Too many to deal with + dontwarn="$dontwarn -Wsign-conversion" # Too many to deal with + dontwarn="$dontwarn -Wvla" # GNULIB gettext.h violates + dontwarn="$dontwarn -Wundef" # Many GNULIB violations + dontwarn="$dontwarn -Wcast-qual" # Need to allow bad cast for execve() + dontwarn="$dontwarn -Wlong-long" # We need to use long long in many places + dontwarn="$dontwarn -Wswitch-default" # We allow manual list of all enum cases without default: + dontwarn="$dontwarn -Wswitch-enum" # We allow optional default: instead of listing all enum values + dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -fstrict-overflow + dontwarn="$dontwarn -Wunsafe-loop-optimizations" # Not a problem since we don't use -funsafe-loop-optimizations
Seems okay (but long lines - any way to break that into 80 columns?)
+ + # We might fundamentally need some of these disabled forever, but ideally + # we'd turn many of them on + dontwarn="$dontwarn -Wformat-nonliteral"
This one may need to always be disabled, thanks to virAsprintf (it would be nicer if we could disable for just one or two files, rather than globally).
+ dontwarn="$dontwarn -Wfloat-equal" + dontwarn="$dontwarn -Wdeclaration-after-statement"
If gcc would do better, I'd love to guarantee C99 and take advantage of this (smaller scopes have maintenance benefits) - thus, I envision this one being permanent (float it up to the earlier set).
+ dontwarn="$dontwarn -Wcast-qual" + dontwarn="$dontwarn -Wconversion" + dontwarn="$dontwarn -Wsign-conversion" + dontwarn="$dontwarn -Wold-style-definition" + dontwarn="$dontwarn -Wmissing-noreturn" + dontwarn="$dontwarn -Wpacked" + dontwarn="$dontwarn -Wunused-macros" + dontwarn="$dontwarn -Woverlength-strings" + dontwarn="$dontwarn -Wmissing-format-attribute" + dontwarn="$dontwarn -Wstack-protector"
Yes, I agree that the rest of these should be temporary, and are probably worth fixing.
+ + # Get all possible GCC warnings + gl_MANYWARN_ALL_GCC([maybewarn]) + + # Remove the ones we don't want, blacklisted earlier + gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn]) + + # Check for $CC support of each warning + for w in $wantwarn; do + gl_WARN_ADD([$w]) + done + + # GNULIB uses '-W' which includes a bunch of stuff, + # kinda like -Wextra. Unfortunately, it means you
"kinda like"? Exactly like! -W was the old spelling, -Wextra was the new one (added quite some time ago; something like gcc 3.4, off the top of my head).
Ah, I didn't realize it was the same.
+ # can't simply use '-Wsign-compare' with gl_MANYWARN_COMPLEMENT + # So we have -W enabled, and then have to explicitly turn off + gl_WARN_ADD(-Wno-sign-compare) + + # This should be < 256 really, but with PATH_MAX everywhere + # we have doom, even with 4096. In fact we have some functions + # with several PATH_MAX sized variables :-( We should kill off + # all PATH_MAX usage and then lower this limit + gl_WARN_ADD([-Wframe-larger-than=65700]) + dnl gl_WARN_ADD([-Wframe-larger-than=4096]) + dnl gl_WARN_ADD([-Wframe-larger-than=256]) + + # Extra special flags + gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) + dnl Fedora only uses -fstack-protector, but doesn't seem to + dnl be great overhead in adding -fstack-protector-all instead + dnl gl_WARN_ADD([-fstack-protector]) + gl_WARN_ADD([-fstack-protector-all]) + gl_WARN_ADD([--param=ssp-buffer-size=4]) + gl_WARN_ADD([-fexceptions]) + gl_WARN_ADD([-fasynchronous-unwind-tables]) + gl_WARN_ADD([-fdiagnostics-show-option]) + + if test "$enable_compile_warnings" = "error" + then + gl_WARN_ADD([-Werror]) + fi
Looks like a good conversion.
I also added gl_WARN_ADD([-Wjump-misses-init]) because we loose that due to turning off -Wc++-compat 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 :|
Split the bit acinclude.m4 file into smaller pieces named as m4/virt-XXXXX.m4 * .gitignore: Ignore gettext related files * acinclude.m4: Delete * m4/virt-compile-warnings.m4: Checks for GCC compiler flags * m4/virt-pkgconfig-back-compat.m4: Backcompat check for pkgconfig program --- .gitignore | 36 +++++++++- acinclude.m4 | 137 -------------------------------------- m4/virt-compile-warnings.m4 | 112 +++++++++++++++++++++++++++++++ m4/virt-pkgconfig-back-compat.m4 | 23 ++++++ 4 files changed, 169 insertions(+), 139 deletions(-) delete mode 100644 acinclude.m4 create mode 100644 m4/virt-compile-warnings.m4 create mode 100644 m4/virt-pkgconfig-back-compat.m4 diff --git a/.gitignore b/.gitignore index ba4d351..278bc4f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ -!/m4/compiler-flags.m4 !/po/*.po !/po/POTFILES.in !/po/libvirt.pot @@ -46,7 +45,6 @@ /libvirt.spec /ltconfig /ltmain.sh -/m4/ /maint.mk /mingw32-libvirt.spec /mkinstalldirs @@ -67,3 +65,37 @@ results.log stamp-h stamp-h.in stamp-h1 +m4/codeset.m4 +m4/gettext.m4 +m4/glibc21.m4 +m4/iconv.m4 +m4/intdiv0.m4 +m4/intmax.m4 +m4/inttypes_h.m4 +m4/inttypes.m4 +m4/inttypes-pri.m4 +m4/isc-posix.m4 +m4/lcmessage.m4 +m4/lib-ld.m4 +m4/lib-link.m4 +m4/lib-prefix.m4 +m4/libtool.m4 +m4/longdouble.m4 +m4/longlong.m4 +m4/lt~obsolete.m4 +m4/ltoptions.m4 +m4/ltsugar.m4 +m4/ltversion.m4 +m4/nls.m4 +m4/po.m4 +m4/printf-posix.m4 +m4/progtest.m4 +m4/signed.m4 +m4/size_max.m4 +m4/stdint_h.m4 +m4/uintmax_t.m4 +m4/ulonglong.m4 +m4/wchar_t.m4 +m4/wint_t.m4 +m4/xsize.m4 +m4/.gitignore diff --git a/acinclude.m4 b/acinclude.m4 deleted file mode 100644 index 8eb0ec5..0000000 --- a/acinclude.m4 +++ /dev/null @@ -1,137 +0,0 @@ -dnl -dnl Enable all known GCC compiler warnings, except for those -dnl we can't yet cope with -dnl -AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ - dnl ****************************** - dnl More compiler warnings - dnl ****************************** - - AC_ARG_ENABLE(compile-warnings, - [AC_HELP_STRING([--enable-compile-warnings=@<:@no/yes/error@:>@], - [Turn on compiler warnings])],, - [enable_compile_warnings="m4_default([$1],[yes])"]) - - case "$enable_compile_warnings" in - no) - try_compiler_flags="" - ;; - yes|minimum|maximum|error) - - # List of warnings that are not relevant / wanted - dontwarn="$dontwarn -Wc++-compat" # Don't care about C++ compiler compat - dontwarn="$dontwarn -Wtraditional" # Don't care about ancient C standard compat - dontwarn="$dontwarn -Wtraditional-conversion" # Don't care about ancient C standard compat - dontwarn="$dontwarn -Wsystem-headers" # Ignore warnings in /usr/include - dontwarn="$dontwarn -Wpadded" # Happy for compiler to add struct padding - dontwarn="$dontwarn -Wunreachable-code" # GCC very confused with -O2 - dontwarn="$dontwarn -Wconversion" # Too many to deal with - dontwarn="$dontwarn -Wsign-conversion" # Too many to deal with - dontwarn="$dontwarn -Wvla" # GNULIB gettext.h violates - dontwarn="$dontwarn -Wundef" # Many GNULIB violations - dontwarn="$dontwarn -Wcast-qual" # Need to allow bad cast for execve() - dontwarn="$dontwarn -Wlong-long" # We need to use long long in many places - dontwarn="$dontwarn -Wswitch-default" # We allow manual list of all enum cases without default: - dontwarn="$dontwarn -Wswitch-enum" # We allow optional default: instead of listing all enum values - dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -fstrict-overflow - dontwarn="$dontwarn -Wunsafe-loop-optimizations" # Not a problem since we don't use -funsafe-loop-optimizations - - # We might fundamentally need some of these disabled forever, but ideally - # we'd turn many of them on - dontwarn="$dontwarn -Wformat-nonliteral" - dontwarn="$dontwarn -Wfloat-equal" - dontwarn="$dontwarn -Wdeclaration-after-statement" - dontwarn="$dontwarn -Wcast-qual" - dontwarn="$dontwarn -Wconversion" - dontwarn="$dontwarn -Wsign-conversion" - dontwarn="$dontwarn -Wold-style-definition" - dontwarn="$dontwarn -Wmissing-noreturn" - dontwarn="$dontwarn -Wpacked" - dontwarn="$dontwarn -Wunused-macros" - dontwarn="$dontwarn -Woverlength-strings" - dontwarn="$dontwarn -Wmissing-format-attribute" - dontwarn="$dontwarn -Wstack-protector" - - # Get all possible GCC warnings - gl_MANYWARN_ALL_GCC([maybewarn]) - - # Remove the ones we don't want, blacklisted earlier - gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn]) - - # Check for $CC support of each warning - for w in $wantwarn; do - gl_WARN_ADD([$w]) - done - - # GNULIB uses '-W' which includes a bunch of stuff, - # kinda like -Wextra. Unfortunately, it means you - # can't simply use '-Wsign-compare' with gl_MANYWARN_COMPLEMENT - # So we have -W enabled, and then have to explicitly turn off - gl_WARN_ADD(-Wno-sign-compare) - - # This should be < 256 really, but with PATH_MAX everywhere - # we have doom, even with 4096. In fact we have some functions - # with several PATH_MAX sized variables :-( We should kill off - # all PATH_MAX usage and then lower this limit - gl_WARN_ADD([-Wframe-larger-than=65700]) - dnl gl_WARN_ADD([-Wframe-larger-than=4096]) - dnl gl_WARN_ADD([-Wframe-larger-than=256]) - - # Extra special flags - gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) - dnl Fedora only uses -fstack-protector, but doesn't seem to - dnl be great overhead in adding -fstack-protector-all instead - dnl gl_WARN_ADD([-fstack-protector]) - gl_WARN_ADD([-fstack-protector-all]) - gl_WARN_ADD([--param=ssp-buffer-size=4]) - gl_WARN_ADD([-fexceptions]) - gl_WARN_ADD([-fasynchronous-unwind-tables]) - gl_WARN_ADD([-fdiagnostics-show-option]) - - if test "$enable_compile_warnings" = "error" - then - gl_WARN_ADD([-Werror]) - fi - ;; - *) - AC_MSG_ERROR(Unknown argument '$enable_compile_warnings' to --enable-compile-warnings) - ;; - esac - - WARN_LDFLAGS=$WARN_CFLAGS - AC_SUBST([WARN_CFLAGS]) - AC_SUBST([WARN_LDFLAGS]) - - dnl Needed to keep compile quiet on python 2.4 - save_WARN_CFLAGS=$WARN_CFLAGS - WARN_CFLAGS= - gl_WARN_ADD(-Wno-redundant-decls) - WARN_PYTHON_CFLAGS=$WARN_CFLAGS - AC_SUBST(WARN_PYTHON_CFLAGS) - WARN_CFLAGS=$save_WARN_CFLAGS -]) - - -dnl -dnl To support the old pkg-config from RHEL4 vintage, we need -dnl to define the PKG_PROG_PKG_CONFIG macro if its not already -dnl present -m4_ifndef([PKG_PROG_PKG_CONFIG], - [AC_DEFUN([PKG_PROG_PKG_CONFIG], -[m4_pattern_forbid([^_?PKG_[A-Z_]+$]) -m4_pattern_allow([^PKG_CONFIG(_PATH)?$]) -AC_ARG_VAR([PKG_CONFIG], [path to pkg-config utility])dnl -if test "x$ac_cv_env_PKG_CONFIG_set" != "xset"; then - AC_PATH_TOOL([PKG_CONFIG], [pkg-config]) -fi -if test -n "$PKG_CONFIG"; then - _pkg_min_version=m4_default([$1], [0.9.0]) - AC_MSG_CHECKING([pkg-config is at least version $_pkg_min_version]) - if $PKG_CONFIG --atleast-pkgconfig-version $_pkg_min_version; then - AC_MSG_RESULT([yes]) - else - AC_MSG_RESULT([no]) - PKG_CONFIG="" - fi -fi[]dnl -])]) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 new file mode 100644 index 0000000..f8ee474 --- /dev/null +++ b/m4/virt-compile-warnings.m4 @@ -0,0 +1,112 @@ +dnl +dnl Enable all known GCC compiler warnings, except for those +dnl we can't yet cope with +dnl +AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ + dnl ****************************** + dnl More compiler warnings + dnl ****************************** + + AC_ARG_ENABLE(compile-warnings, + [AC_HELP_STRING([--enable-compile-warnings=@<:@no/yes/error@:>@], + [Turn on compiler warnings])],, + [enable_compile_warnings="m4_default([$1],[yes])"]) + + case "$enable_compile_warnings" in + no) + try_compiler_flags="" + ;; + yes|minimum|maximum|error) + + # List of warnings that are not relevant / wanted + dontwarn="$dontwarn -Wc++-compat" # Don't care about C++ compiler compat + dontwarn="$dontwarn -Wtraditional" # Don't care about ancient C standard compat + dontwarn="$dontwarn -Wtraditional-conversion" # Don't care about ancient C standard compat + dontwarn="$dontwarn -Wsystem-headers" # Ignore warnings in /usr/include + dontwarn="$dontwarn -Wpadded" # Happy for compiler to add struct padding + dontwarn="$dontwarn -Wunreachable-code" # GCC very confused with -O2 + dontwarn="$dontwarn -Wconversion" # Too many to deal with + dontwarn="$dontwarn -Wsign-conversion" # Too many to deal with + dontwarn="$dontwarn -Wvla" # GNULIB gettext.h violates + dontwarn="$dontwarn -Wundef" # Many GNULIB violations + dontwarn="$dontwarn -Wcast-qual" # Need to allow bad cast for execve() + dontwarn="$dontwarn -Wlong-long" # We need to use long long in many places + dontwarn="$dontwarn -Wswitch-default" # We allow manual list of all enum cases without default: + dontwarn="$dontwarn -Wswitch-enum" # We allow optional default: instead of listing all enum values + dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -fstrict-overflow + dontwarn="$dontwarn -Wunsafe-loop-optimizations" # Not a problem since we don't use -funsafe-loop-optimizations + + # We might fundamentally need some of these disabled forever, but ideally + # we'd turn many of them on + dontwarn="$dontwarn -Wformat-nonliteral" + dontwarn="$dontwarn -Wfloat-equal" + dontwarn="$dontwarn -Wdeclaration-after-statement" + dontwarn="$dontwarn -Wcast-qual" + dontwarn="$dontwarn -Wconversion" + dontwarn="$dontwarn -Wsign-conversion" + dontwarn="$dontwarn -Wold-style-definition" + dontwarn="$dontwarn -Wmissing-noreturn" + dontwarn="$dontwarn -Wpacked" + dontwarn="$dontwarn -Wunused-macros" + dontwarn="$dontwarn -Woverlength-strings" + dontwarn="$dontwarn -Wmissing-format-attribute" + dontwarn="$dontwarn -Wstack-protector" + + # Get all possible GCC warnings + gl_MANYWARN_ALL_GCC([maybewarn]) + + # Remove the ones we don't want, blacklisted earlier + gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn]) + + # Check for $CC support of each warning + for w in $wantwarn; do + gl_WARN_ADD([$w]) + done + + # GNULIB uses '-W' which includes a bunch of stuff, + # kinda like -Wextra. Unfortunately, it means you + # can't simply use '-Wsign-compare' with gl_MANYWARN_COMPLEMENT + # So we have -W enabled, and then have to explicitly turn off + gl_WARN_ADD(-Wno-sign-compare) + + # This should be < 256 really, but with PATH_MAX everywhere + # we have doom, even with 4096. In fact we have some functions + # with several PATH_MAX sized variables :-( We should kill off + # all PATH_MAX usage and then lower this limit + gl_WARN_ADD([-Wframe-larger-than=65700]) + dnl gl_WARN_ADD([-Wframe-larger-than=4096]) + dnl gl_WARN_ADD([-Wframe-larger-than=256]) + + # Extra special flags + gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) + dnl Fedora only uses -fstack-protector, but doesn't seem to + dnl be great overhead in adding -fstack-protector-all instead + dnl gl_WARN_ADD([-fstack-protector]) + gl_WARN_ADD([-fstack-protector-all]) + gl_WARN_ADD([--param=ssp-buffer-size=4]) + gl_WARN_ADD([-fexceptions]) + gl_WARN_ADD([-fasynchronous-unwind-tables]) + gl_WARN_ADD([-fdiagnostics-show-option]) + + if test "$enable_compile_warnings" = "error" + then + gl_WARN_ADD([-Werror]) + fi + ;; + *) + AC_MSG_ERROR(Unknown argument '$enable_compile_warnings' to --enable-compile-warnings) + ;; + esac + + WARN_LDFLAGS=$WARN_CFLAGS + AC_SUBST([WARN_CFLAGS]) + AC_SUBST([WARN_LDFLAGS]) + + dnl Needed to keep compile quiet on python 2.4 + save_WARN_CFLAGS=$WARN_CFLAGS + WARN_CFLAGS= + gl_WARN_ADD(-Wno-redundant-decls) + WARN_PYTHON_CFLAGS=$WARN_CFLAGS + AC_SUBST(WARN_PYTHON_CFLAGS) + WARN_CFLAGS=$save_WARN_CFLAGS +]) diff --git a/m4/virt-pkgconfig-back-compat.m4 b/m4/virt-pkgconfig-back-compat.m4 new file mode 100644 index 0000000..7eab84b --- /dev/null +++ b/m4/virt-pkgconfig-back-compat.m4 @@ -0,0 +1,23 @@ +dnl +dnl To support the old pkg-config from RHEL4 vintage, we need +dnl to define the PKG_PROG_PKG_CONFIG macro if its not already +dnl present +m4_ifndef([PKG_PROG_PKG_CONFIG], + [AC_DEFUN([PKG_PROG_PKG_CONFIG], +[m4_pattern_forbid([^_?PKG_[A-Z_]+$]) +m4_pattern_allow([^PKG_CONFIG(_PATH)?$]) +AC_ARG_VAR([PKG_CONFIG], [path to pkg-config utility])dnl +if test "x$ac_cv_env_PKG_CONFIG_set" != "xset"; then + AC_PATH_TOOL([PKG_CONFIG], [pkg-config]) +fi +if test -n "$PKG_CONFIG"; then + _pkg_min_version=m4_default([$1], [0.9.0]) + AC_MSG_CHECKING([pkg-config is at least version $_pkg_min_version]) + if $PKG_CONFIG --atleast-pkgconfig-version $_pkg_min_version; then + AC_MSG_RESULT([yes]) + else + AC_MSG_RESULT([no]) + PKG_CONFIG="" + fi +fi[]dnl +])]) -- 1.7.4
On 04/04/2011 10:20 AM, Daniel P. Berrange wrote:
Split the bit acinclude.m4 file into smaller pieces named as m4/virt-XXXXX.m4
* .gitignore: Ignore gettext related files * acinclude.m4: Delete * m4/virt-compile-warnings.m4: Checks for GCC compiler flags * m4/virt-pkgconfig-back-compat.m4: Backcompat check for pkgconfig program --- .gitignore | 36 +++++++++- acinclude.m4 | 137 -------------------------------------- m4/virt-compile-warnings.m4 | 112 +++++++++++++++++++++++++++++++ m4/virt-pkgconfig-back-compat.m4 | 23 ++++++ 4 files changed, 169 insertions(+), 139 deletions(-) delete mode 100644 acinclude.m4 create mode 100644 m4/virt-compile-warnings.m4 create mode 100644 m4/virt-pkgconfig-back-compat.m4
diff --git a/.gitignore b/.gitignore index ba4d351..278bc4f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ -!/m4/compiler-flags.m4 !/po/*.po !/po/POTFILES.in !/po/libvirt.pot @@ -46,7 +45,6 @@ /libvirt.spec
Yuck - gnulib's bootstrap script sorts .gitignore, but this is broken (lines starting with ! should come last, not first, according to 'man gitignore'; it's a shame I'm just noticing it now, since it's been broken since November 2010).
@@ -67,3 +65,37 @@ results.log stamp-h stamp-h.in stamp-h1 +m4/codeset.m4 +m4/gettext.m4
Rerunning bootstrap will re-sort this list, leading to spurious diffs unless we get it sorted first. But why should we blacklist all of the m4 files individually, when it is much easier to whitelist just the files we want? /m4/ !/m4/virt-*.m4 I've posted an upstream bug to gnulib about bootstrap botching sort order; let's get that fixed first and then respin this patch on top of that one. However, even though it's not ready for prime-time yet, I'm pleased with the concept of splitting things into easier-to-find per-file macros. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Apr 04, 2011 at 02:37:00PM -0600, Eric Blake wrote:
On 04/04/2011 10:20 AM, Daniel P. Berrange wrote:
Split the bit acinclude.m4 file into smaller pieces named as m4/virt-XXXXX.m4
* .gitignore: Ignore gettext related files * acinclude.m4: Delete * m4/virt-compile-warnings.m4: Checks for GCC compiler flags * m4/virt-pkgconfig-back-compat.m4: Backcompat check for pkgconfig program --- .gitignore | 36 +++++++++- acinclude.m4 | 137 -------------------------------------- m4/virt-compile-warnings.m4 | 112 +++++++++++++++++++++++++++++++ m4/virt-pkgconfig-back-compat.m4 | 23 ++++++ 4 files changed, 169 insertions(+), 139 deletions(-) delete mode 100644 acinclude.m4 create mode 100644 m4/virt-compile-warnings.m4 create mode 100644 m4/virt-pkgconfig-back-compat.m4
diff --git a/.gitignore b/.gitignore index ba4d351..278bc4f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ -!/m4/compiler-flags.m4 !/po/*.po !/po/POTFILES.in !/po/libvirt.pot @@ -46,7 +45,6 @@ /libvirt.spec
Yuck - gnulib's bootstrap script sorts .gitignore, but this is broken (lines starting with ! should come last, not first, according to 'man gitignore'; it's a shame I'm just noticing it now, since it's been broken since November 2010).
@@ -67,3 +65,37 @@ results.log stamp-h stamp-h.in stamp-h1 +m4/codeset.m4 +m4/gettext.m4
Rerunning bootstrap will re-sort this list, leading to spurious diffs unless we get it sorted first. But why should we blacklist all of the m4 files individually, when it is much easier to whitelist just the files we want?
/m4/ !/m4/virt-*.m4
Ok, I've pushed with that, but with the bogus sort order, since it is no worse than what we already have. We can fix sort order once gnulib is fixed. 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 :|
Add a couple of missing ATTRIBUTE_FMT_PRINTF annotations * tools/virsh.c, tests/testutils.c: Add printf format attribute * m4/virt-compile-warnings.m4: Enable -Wmissing-format-attribute --- m4/virt-compile-warnings.m4 | 1 - tests/testutils.c | 2 +- tools/virsh.c | 3 ++- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index f8ee474..51e21a9 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -49,7 +49,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wpacked" dontwarn="$dontwarn -Wunused-macros" dontwarn="$dontwarn -Woverlength-strings" - dontwarn="$dontwarn -Wmissing-format-attribute" dontwarn="$dontwarn -Wstack-protector" # Get all possible GCC warnings diff --git a/tests/testutils.c b/tests/testutils.c index 3110457..0ce3c7b 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -67,7 +67,7 @@ virtTestCountAverage(double *items, int nitems) return (double) (sum / nitems); } - +ATTRIBUTE_FMT_PRINTF(3,4) void virtTestResult(const char *name, int ret, const char *msg, ...) { va_list vargs; diff --git a/tools/virsh.c b/tools/virsh.c index 19e3449..4765f5c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -243,7 +243,8 @@ static int vshInit(vshControl *ctl); static int vshDeinit(vshControl *ctl); static void vshUsage(void); static void vshOpenLogFile(vshControl *ctl); -static void vshOutputLogFile(vshControl *ctl, int log_level, const char *format, va_list ap); +static void vshOutputLogFile(vshControl *ctl, int log_level, const char *format, va_list ap) + ATTRIBUTE_FMT_PRINTF(3, 0); static void vshCloseLogFile(vshControl *ctl); static int vshParseArgv(vshControl *ctl, int argc, char **argv); -- 1.7.4
On 04/04/2011 10:20 AM, Daniel P. Berrange wrote:
Add a couple of missing ATTRIBUTE_FMT_PRINTF annotations
* tools/virsh.c, tests/testutils.c: Add printf format attribute * m4/virt-compile-warnings.m4: Enable -Wmissing-format-attribute
- +ATTRIBUTE_FMT_PRINTF(3,4) void virtTestResult(const char *name, int ret, const char *msg, ...)
gcc attribute syntax is screwy. I'm somewhat used to: void ATTRIBUTE_FMT_PRINTF(3,4) virtTestResult(...) but if doing the attribute before the return type works, great.
{ va_list vargs; diff --git a/tools/virsh.c b/tools/virsh.c index 19e3449..4765f5c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -243,7 +243,8 @@ static int vshInit(vshControl *ctl); static int vshDeinit(vshControl *ctl); static void vshUsage(void); static void vshOpenLogFile(vshControl *ctl); -static void vshOutputLogFile(vshControl *ctl, int log_level, const char *format, va_list ap); +static void vshOutputLogFile(vshControl *ctl, int log_level, const char *format, va_list ap) + ATTRIBUTE_FMT_PRINTF(3, 0); static void vshCloseLogFile(vshControl *ctl);
ACK (although rebasing this to apply before we fix patch 2/6 vs. gnulib bootstrap may not be worth your time). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* src/internal.h: Define a ATTRIBUTE_NO_RETURN annotation * src/lxc/lxc_container.c: Annotate lxcContainerDummyChild with ATTRIBUTE_NO_RETURN * tests/eventtest.c: Mark async thread as ATTRIBUTE_NO_RETURN * m4/virt-compile-warnings.m4: Enable -Wmissing-noreturn --- m4/virt-compile-warnings.m4 | 1 - src/internal.h | 9 +++++++++ src/lxc/lxc_container.c | 3 ++- tests/eventtest.c | 3 +-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 51e21a9..9643419 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -45,7 +45,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wconversion" dontwarn="$dontwarn -Wsign-conversion" dontwarn="$dontwarn -Wold-style-definition" - dontwarn="$dontwarn -Wmissing-noreturn" dontwarn="$dontwarn -Wpacked" dontwarn="$dontwarn -Wunused-macros" dontwarn="$dontwarn -Woverlength-strings" diff --git a/src/internal.h b/src/internal.h index 2afbd8d..4641fc1 100644 --- a/src/internal.h +++ b/src/internal.h @@ -117,6 +117,15 @@ # endif /** + * ATTRIBUTE_NORETURN: + * + * Macro to indicate that a function won't return to the caller + */ +# ifndef ATTRIBUTE_NORETURN +# define ATTRIBUTE_NORETURN __attribute__((__noreturn__)) +# endif + +/** * ATTRIBUTE_SENTINEL: * * Macro to check for NULL-terminated varargs lists diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 9830b71..af453f3 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -914,7 +914,8 @@ int lxcContainerStart(virDomainDefPtr def, return pid; } -static int lxcContainerDummyChild(void *argv ATTRIBUTE_UNUSED) +ATTRIBUTE_NORETURN static int +lxcContainerDummyChild(void *argv ATTRIBUTE_UNUSED) { _exit(0); } diff --git a/tests/eventtest.c b/tests/eventtest.c index 4d22070..eb4b755 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -119,7 +119,7 @@ static pthread_cond_t eventThreadJobCond = PTHREAD_COND_INITIALIZER; static int eventThreadJobDone = 0; -static void *eventThreadLoop(void *data ATTRIBUTE_UNUSED) { +ATTRIBUTE_NORETURN static void *eventThreadLoop(void *data ATTRIBUTE_UNUSED) { while (1) { pthread_mutex_lock(&eventThreadMutex); while (!eventThreadRunOnce) { @@ -135,7 +135,6 @@ static void *eventThreadLoop(void *data ATTRIBUTE_UNUSED) { pthread_cond_signal(&eventThreadJobCond); pthread_mutex_unlock(&eventThreadMutex); } - return NULL; } -- 1.7.4
On 04/04/2011 10:20 AM, Daniel P. Berrange wrote:
* src/internal.h: Define a ATTRIBUTE_NO_RETURN annotation * src/lxc/lxc_container.c: Annotate lxcContainerDummyChild with ATTRIBUTE_NO_RETURN * tests/eventtest.c: Mark async thread as ATTRIBUTE_NO_RETURN * m4/virt-compile-warnings.m4: Enable -Wmissing-noreturn --- m4/virt-compile-warnings.m4 | 1 - src/internal.h | 9 +++++++++ src/lxc/lxc_container.c | 3 ++- tests/eventtest.c | 3 +-- 4 files changed, 12 insertions(+), 4 deletions(-)
+++ b/src/internal.h @@ -117,6 +117,15 @@ # endif
/** + * ATTRIBUTE_NORETURN: + * + * Macro to indicate that a function won't return to the caller + */ +# ifndef ATTRIBUTE_NORETURN +# define ATTRIBUTE_NORETURN __attribute__((__noreturn__)) +# endif
Do we need a minimum gcc version detection, so this cause grief on older setups? At any rate, ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Apr 04, 2011 at 02:42:18PM -0600, Eric Blake wrote:
On 04/04/2011 10:20 AM, Daniel P. Berrange wrote:
* src/internal.h: Define a ATTRIBUTE_NO_RETURN annotation * src/lxc/lxc_container.c: Annotate lxcContainerDummyChild with ATTRIBUTE_NO_RETURN * tests/eventtest.c: Mark async thread as ATTRIBUTE_NO_RETURN * m4/virt-compile-warnings.m4: Enable -Wmissing-noreturn --- m4/virt-compile-warnings.m4 | 1 - src/internal.h | 9 +++++++++ src/lxc/lxc_container.c | 3 ++- tests/eventtest.c | 3 +-- 4 files changed, 12 insertions(+), 4 deletions(-)
+++ b/src/internal.h @@ -117,6 +117,15 @@ # endif
/** + * ATTRIBUTE_NORETURN: + * + * Macro to indicate that a function won't return to the caller + */ +# ifndef ATTRIBUTE_NORETURN +# define ATTRIBUTE_NORETURN __attribute__((__noreturn__)) +# endif
Do we need a minimum gcc version detection, so this cause grief on older setups?
It has existed since gcc 2.4, so IMHO that is so old we don't need the check 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 :|
A couple of functions were declared using the old style foo() for no-parameters, instead of foo(void) * src/xen/xen_hypervisor.c, tests/testutils.c: Replace () with (void) in some function declarations * m4/virt-compile-warnings.m4: Enable -Wold-style-definition --- m4/virt-compile-warnings.m4 | 1 - src/xen/xen_hypervisor.c | 2 +- tests/testutils.c | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 9643419..24710e6 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -44,7 +44,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wcast-qual" dontwarn="$dontwarn -Wconversion" dontwarn="$dontwarn -Wsign-conversion" - dontwarn="$dontwarn -Wold-style-definition" dontwarn="$dontwarn -Wpacked" dontwarn="$dontwarn -Wunused-macros" dontwarn="$dontwarn -Woverlength-strings" diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 47355ce..8a9dae5 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -3621,7 +3621,7 @@ xenHypervisorGetVcpuMax(virDomainPtr domain) * Return true if the current process should be able to connect to Xen. */ int -xenHavePrivilege() +xenHavePrivilege(void) { #ifdef __sun return priv_ineffect (PRIV_XVM_CONTROL); diff --git a/tests/testutils.c b/tests/testutils.c index 0ce3c7b..1f3b569 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -457,14 +457,14 @@ virTestGetFlag(const char *name) { } unsigned int -virTestGetDebug() { +virTestGetDebug(void) { if (testDebug == -1) testDebug = virTestGetFlag("VIR_TEST_DEBUG"); return testDebug; } unsigned int -virTestGetVerbose() { +virTestGetVerbose(void) { if (testVerbose == -1) testVerbose = virTestGetFlag("VIR_TEST_VERBOSE"); return testVerbose || virTestGetDebug(); -- 1.7.4
On 04/04/2011 10:20 AM, Daniel P. Berrange wrote:
A couple of functions were declared using the old style foo() for no-parameters, instead of foo(void)
* src/xen/xen_hypervisor.c, tests/testutils.c: Replace () with (void) in some function declarations * m4/virt-compile-warnings.m4: Enable -Wold-style-definition --- m4/virt-compile-warnings.m4 | 1 - src/xen/xen_hypervisor.c | 2 +- tests/testutils.c | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* m4/virt-compile-warnings.m4: Enable -Wunused-macros * daemon/libvirtd.c: Remove MAX_LISTEN * daemon/remote.c: Remote VIR_FROM_THIS * examples/domain-events/events-c/event-test.c: Remove VIR_DEBUG * python/libvirt-override.c: Put NAME() inside DEBUG_ERROR * src/esx/esx*.c: Comment out unused VIR_FROM_THIS * src/node_device/node_device_hal.c: Remove pointless macros for accessing privateData * src/openvz/openvz_driver.c: Remove CMDBUF_LEN/CMDOP_LEN * src/qemu/qemu_monitor_text.c: Remove QEMU_CMD_PROMPT and QEMU_PASSWD_PROMPT * src/remote/remote_driver.c: Remove UNIX_PATH_MAX * src/security/security_stack.c: Remove VIR_FROM_THIS * src/uml/uml_conf.c: Remove umlLog() * src/uml/uml_driver.c: Remove TEMPDIR * src/util/bridge.c: Remove JIFFIES_TO_MS/MS_TO_JIFFIES * src/util/hooks.c: Ensure VIR_FROM_THIS is used * src/util/logging.c: Remove VIR_FROM_THIS * src/util/macvtap.c: Disable unused constants * src/util/storage_file.c: Disable QCOW1_HDR_TOTAL_SIZE * src/vbox/vbox_driver.c: Remove duplicated VIR_FROM_THIS and make sure it is used * src/util/pci.c: Disable some unused constants * src/xen/xen_hypervisor.c: Remove XEN_V0_IOCTL_HYPERCALL_CMD and unused DOMFLAGS_CPUMASK/DOMFLAGS_CPUSHIFT * src/xen/xm_internal.c: Remove XM_XML_ERROR, XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU and XEND_CONFIG_MIN_VERS_PVFB_NEWCONF * tools/virsh.c: Remove DIR_MODE/LOCK_MODE, LVL_NOTICE constants * tests/sockettest.c: Remove DO_TEST_PARSE --- daemon/libvirtd.c | 2 +- daemon/remote.c | 1 - examples/domain-events/events-c/event-test.c | 2 -- m4/virt-compile-warnings.m4 | 1 - python/libvirt-override.c | 2 ++ src/driver.c | 3 +-- src/esx/esx_device_monitor.c | 2 +- src/esx/esx_interface_driver.c | 2 +- src/esx/esx_network_driver.c | 2 +- src/esx/esx_nwfilter_driver.c | 2 +- src/esx/esx_secret_driver.c | 2 +- src/esx/esx_vi.c | 7 ------- src/esx/esx_vi_methods.c | 9 +++++---- src/esx/esx_vi_types.c | 3 ++- src/libvirt.c | 3 --- src/node_device/node_device_hal.c | 17 ++++------------- src/openvz/openvz_driver.c | 2 -- src/qemu/qemu_monitor_text.c | 3 --- src/remote/remote_driver.c | 3 --- src/security/security_stack.c | 2 -- src/uml/uml_conf.c | 2 -- src/uml/uml_driver.c | 3 --- src/util/bridge.c | 3 --- src/util/hooks.c | 2 +- src/util/logging.c | 2 -- src/util/macvtap.c | 6 ++++-- src/util/pci.c | 4 ++-- src/util/storage_file.c | 4 +++- src/vbox/vbox_driver.c | 4 +--- src/xen/xen_hypervisor.c | 14 +++++--------- src/xen/xm_internal.c | 9 --------- tests/sockettest.c | 10 ---------- tools/virsh.c | 13 ++----------- 33 files changed, 38 insertions(+), 108 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 024f56f..991aaeb 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -3154,7 +3154,7 @@ enum { OPT_VERSION = 129 }; -#define MAX_LISTEN 5 + int main(int argc, char **argv) { struct qemud_server *server = NULL; const char *pid_file = NULL; diff --git a/daemon/remote.c b/daemon/remote.c index 1700c2d..98279e1 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -62,7 +62,6 @@ #include "libvirt/libvirt-qemu.h" #include "command.h" -#define VIR_FROM_THIS VIR_FROM_REMOTE #define REMOTE_DEBUG(fmt, ...) VIR_DEBUG(fmt, __VA_ARGS__) static virDomainPtr get_nonnull_domain (virConnectPtr conn, remote_nonnull_domain domain); diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index 1f46d42..3de2aa8 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -10,8 +10,6 @@ #define VIR_DEBUG0(fmt) printf("%s:%d :: " fmt "\n", \ __func__, __LINE__) -#define VIR_DEBUG(fmt, ...) printf("%s:%d: " fmt "\n", \ - __func__, __LINE__, __VA_ARGS__) #define STREQ(a,b) (strcmp(a,b) == 0) #ifndef ATTRIBUTE_UNUSED diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 24710e6..5022676 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -45,7 +45,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wconversion" dontwarn="$dontwarn -Wsign-conversion" dontwarn="$dontwarn -Wpacked" - dontwarn="$dontwarn -Wunused-macros" dontwarn="$dontwarn -Woverlength-strings" dontwarn="$dontwarn -Wstack-protector" diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 4a9b432..8568ff2 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2595,7 +2595,9 @@ static char *updateTimeoutName = NULL; static PyObject *removeTimeoutObj = NULL; static char *removeTimeoutName = NULL; +#if DEBUG_ERROR #define NAME(fn) ( fn ## Name ? fn ## Name : # fn ) +#endif static int libvirt_virEventAddHandleFunc (int fd, diff --git a/src/driver.c b/src/driver.c index f882ea1..25623cd 100644 --- a/src/driver.c +++ b/src/driver.c @@ -30,13 +30,12 @@ #include "util.h" #include "configmake.h" -#define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver" - /* Make sure ... INTERNAL_CALL can not be set by the caller */ verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); #ifdef WITH_DRIVER_MODULES +# define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver" /* XXX re-implment this for other OS, or use libtools helper lib ? */ diff --git a/src/esx/esx_device_monitor.c b/src/esx/esx_device_monitor.c index d559f96..6d765ac 100644 --- a/src/esx/esx_device_monitor.c +++ b/src/esx/esx_device_monitor.c @@ -35,7 +35,7 @@ #include "esx_vi_methods.h" #include "esx_util.h" -#define VIR_FROM_THIS VIR_FROM_ESX +/* #define VIR_FROM_THIS VIR_FROM_ESX */ diff --git a/src/esx/esx_interface_driver.c b/src/esx/esx_interface_driver.c index 4bac3d5..8d0b9a2 100644 --- a/src/esx/esx_interface_driver.c +++ b/src/esx/esx_interface_driver.c @@ -35,7 +35,7 @@ #include "esx_vi_methods.h" #include "esx_util.h" -#define VIR_FROM_THIS VIR_FROM_ESX +/* #define VIR_FROM_THIS VIR_FROM_ESX */ diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index a64bb8e..7117a60 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -35,7 +35,7 @@ #include "esx_vi_methods.h" #include "esx_util.h" -#define VIR_FROM_THIS VIR_FROM_ESX +/* #define VIR_FROM_THIS VIR_FROM_ESX */ diff --git a/src/esx/esx_nwfilter_driver.c b/src/esx/esx_nwfilter_driver.c index a9d046d..4ba9f46 100644 --- a/src/esx/esx_nwfilter_driver.c +++ b/src/esx/esx_nwfilter_driver.c @@ -34,7 +34,7 @@ #include "esx_vi_methods.h" #include "esx_util.h" -#define VIR_FROM_THIS VIR_FROM_ESX +/* #define VIR_FROM_THIS VIR_FROM_ESX */ diff --git a/src/esx/esx_secret_driver.c b/src/esx/esx_secret_driver.c index 1ae7ddc..350d8e0 100644 --- a/src/esx/esx_secret_driver.c +++ b/src/esx/esx_secret_driver.c @@ -34,7 +34,7 @@ #include "esx_vi_methods.h" #include "esx_util.h" -#define VIR_FROM_THIS VIR_FROM_ESX +/* #define VIR_FROM_THIS VIR_FROM_ESX */ diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 7446ec5..3b38937 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -40,13 +40,6 @@ #define VIR_FROM_THIS VIR_FROM_ESX - -#define ESX_VI__SOAP__RESPONSE_XPATH(_type) \ - ((char *)"/soapenv:Envelope/soapenv:Body/" \ - "vim:"_type"Response/vim:returnval") - - - #define ESX_VI__TEMPLATE__ALLOC(_type) \ int \ esxVI_##_type##_Alloc(esxVI_##_type **ptrptr) \ diff --git a/src/esx/esx_vi_methods.c b/src/esx/esx_vi_methods.c index 5967088..440a62a 100644 --- a/src/esx/esx_vi_methods.c +++ b/src/esx/esx_vi_methods.c @@ -1,4 +1,3 @@ - /* * esx_vi_methods.c: client for the VMware VI API 2.5 to manage ESX hosts * @@ -52,8 +51,10 @@ +#if 0 #define ESX_VI__METHOD__CHECK_OUTPUT__RequiredList \ ESX_VI__METHOD__CHECK_OUTPUT__NotNone +#endif @@ -78,12 +79,12 @@ } - -#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__RequiredList(_type, _suffix) \ +#if 0 +#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__RequiredList(_type) \ if (esxVI_##_type##_DeserializeList(response->node, output) < 0) { \ goto cleanup; \ } - +#endif #define ESX_VI__METHOD__DESERIALIZE_OUTPUT__OptionalItem(_type, _suffix) \ diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index f3df2b5..56121ef 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -531,11 +531,12 @@ +#if 0 #define ESX_VI__TEMPLATE__DISPATCH__DEEP_COPY(_type) \ case esxVI_Type_##_type: \ return esxVI_##_type##_DeepCopy((esxVI_##_type **)dst, \ (esxVI_##_type *)src); - +#endif #define ESX_VI__TEMPLATE__DISPATCH__CAST_FROM_ANY_TYPE(_type) \ diff --git a/src/libvirt.c b/src/libvirt.c index 8be18d4..564988b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -492,9 +492,6 @@ DllMain (HINSTANCE instance ATTRIBUTE_UNUSED, #define virLibSecretError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_SECRET, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) -#define virLibStreamError(code, ...) \ - virReportErrorHelper(NULL, VIR_FROM_STREAMS, code, __FILE__, \ - __FUNCTION__, __LINE__, __VA_ARGS__) #define virLibNWFilterError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_NWFILTER, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 3af2bcf..b4a4c73 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -38,21 +38,12 @@ #include "logging.h" #include "node_device_driver.h" -#define VIR_FROM_THIS VIR_FROM_NODEDEV - /* * Host device enumeration (HAL implementation) */ static virDeviceMonitorStatePtr driverState; -#define CONN_DRV_STATE(conn) \ - ((virDeviceMonitorStatePtr)((conn)->devMonPrivateData)) -#define DRV_STATE_HAL_CTX(ds) ((LibHalContext *)((ds)->privateData)) -#define CONN_HAL_CTX(conn) DRV_STATE_HAL_CTX(CONN_DRV_STATE(conn)) - -#define NODE_DEV_UDI(obj) ((const char *)((obj)->privateData) - static const char *hal_name(const char *udi) { @@ -441,7 +432,7 @@ static void dev_create(const char *udi) return; nodeDeviceLock(driverState); - ctx = DRV_STATE_HAL_CTX(driverState); + ctx = driverState->privateData; if (VIR_ALLOC(def) < 0) goto failure; @@ -600,7 +591,7 @@ static void dbus_watch_callback(int fdatch ATTRIBUTE_UNUSED, (void)dbus_watch_handle(watch, dbus_flags); nodeDeviceLock(driverState); - hal_ctx = DRV_STATE_HAL_CTX(driverState); + hal_ctx = driverState->privateData; dbus_conn = libhal_ctx_get_dbus_connection(hal_ctx); nodeDeviceUnlock(driverState); while (dbus_connection_dispatch(dbus_conn) == DBUS_DISPATCH_DATA_REMAINS) @@ -808,7 +799,7 @@ static int halDeviceMonitorShutdown(void) { if (driverState) { nodeDeviceLock(driverState); - LibHalContext *hal_ctx = DRV_STATE_HAL_CTX(driverState); + LibHalContext *hal_ctx = driverState->privateData; virNodeDeviceObjListFree(&driverState->devs); (void)libhal_ctx_shutdown(hal_ctx, NULL); (void)libhal_ctx_free(hal_ctx); @@ -834,7 +825,7 @@ static int halDeviceMonitorReload(void) virNodeDeviceObjListFree(&driverState->devs); nodeDeviceUnlock(driverState); - hal_ctx = DRV_STATE_HAL_CTX(driverState); + hal_ctx = driverState->privateData; VIR_INFO0("Creating new objects"); dbus_error_init(&err); udi = libhal_get_all_devices(hal_ctx, &num_devs, &err); diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index fb30c37..c492341 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -63,8 +63,6 @@ #define VIR_FROM_THIS VIR_FROM_OPENVZ #define OPENVZ_MAX_ARG 28 -#define CMDBUF_LEN 1488 -#define CMDOP_LEN 288 static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid); static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 168c60f..2208573 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -43,9 +43,6 @@ #define VIR_FROM_THIS VIR_FROM_QEMU -#define QEMU_CMD_PROMPT "\n(qemu) " -#define QEMU_PASSWD_PROMPT "Password: " - #define DEBUG_IO 0 /* Return -1 for error, 0 for success */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index bf94e70..7f7687a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -691,9 +691,6 @@ doRemoteOpen (virConnectPtr conn, } } -# ifndef UNIX_PATH_MAX -# define UNIX_PATH_MAX(addr) (sizeof (addr).sun_path) -# endif struct sockaddr_un addr; int trials = 0; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 64f745a..5cdcc6a 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -24,8 +24,6 @@ #include "virterror_internal.h" -#define VIR_FROM_THIS VIR_FROM_SECURITY - typedef struct _virSecurityStackData virSecurityStackData; typedef virSecurityStackData *virSecurityStackDataPtr; diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 7c8fb16..909515f 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -52,8 +52,6 @@ #define VIR_FROM_THIS VIR_FROM_UML -#define umlLog(level, msg, ...) \ - virLogMessage(__FILE__, level, 0, msg, __VA_ARGS__) virCapsPtr umlCapsInit(void) { struct utsname utsname; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index e2bd5f2..a26087c 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -65,9 +65,6 @@ #define VIR_FROM_THIS VIR_FROM_UML -/* For storing short-lived temporary files. */ -#define TEMPDIR LOCALSTATEDIR "/cache/libvirt" - typedef struct _umlDomainObjPrivate umlDomainObjPrivate; typedef umlDomainObjPrivate *umlDomainObjPrivatePtr; struct _umlDomainObjPrivate { diff --git a/src/util/bridge.c b/src/util/bridge.c index 3ed71be..00234df 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -52,9 +52,6 @@ # include "logging.h" # include "network.h" -# define JIFFIES_TO_MS(j) (((j)*1000)/HZ) -# define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000) - struct _brControl { int fd; }; diff --git a/src/util/hooks.c b/src/util/hooks.c index 819c95c..3eeb698 100644 --- a/src/util/hooks.c +++ b/src/util/hooks.c @@ -42,7 +42,7 @@ #define VIR_FROM_THIS VIR_FROM_HOOK #define virHookReportError(code, ...) \ - virReportErrorHelper(NULL, VIR_FROM_HOOK, code, __FILE__, \ + virReportErrorHelper(NULL, VIR_FROM_THIS, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) #define LIBVIRT_HOOK_DIR SYSCONFDIR "/libvirt/hooks" diff --git a/src/util/logging.c b/src/util/logging.c index 9d18752..0330fec 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -44,8 +44,6 @@ #include "threads.h" #include "files.h" -#define VIR_FROM_THIS VIR_FROM_NONE - /* * Macro used to format the message as a string in virLogMessage * and borrowed from libxml2 (also used in virRaiseError) diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 346eaf6..f563c23 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -71,8 +71,10 @@ # define MICROSEC_PER_SEC (1000 * 1000) -# define NLMSGBUF_SIZE 256 -# define RATTBUF_SIZE 64 +# if 0 +# define NLMSGBUF_SIZE 256 +# define RATTBUF_SIZE 64 +# endif # define NETLINK_ACK_TIMEOUT_S 2 diff --git a/src/util/pci.c b/src/util/pci.c index 8d2dbb0..0e8cdee 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -96,7 +96,7 @@ struct _pciDeviceList { #define PCI_HEADER_TYPE 0x0e /* Header type */ #define PCI_HEADER_TYPE_BRIDGE 0x1 #define PCI_HEADER_TYPE_MASK 0x7f -#define PCI_HEADER_TYPE_MULTI 0x80 +/*#define PCI_HEADER_TYPE_MULTI 0x80*/ /* PCI30 6.2.1 Device Identification */ #define PCI_CLASS_DEVICE 0x0a /* Device class */ @@ -123,7 +123,7 @@ struct _pciDeviceList { #define PCI_EXP_DEVCAP_FLR (1<<28) /* Function Level Reset */ /* Header type 1 BR12 3.2 PCI-to-PCI Bridge Configuration Space Header Format */ -#define PCI_PRIMARY_BUS 0x18 /* BR12 3.2.5.2 Primary bus number */ +/*#define PCI_PRIMARY_BUS 0x18*/ /* BR12 3.2.5.2 Primary bus number */ #define PCI_SECONDARY_BUS 0x19 /* BR12 3.2.5.3 Secondary bus number */ #define PCI_SUBORDINATE_BUS 0x1a /* BR12 3.2.5.4 Highest bus number behind the bridge */ #define PCI_BRIDGE_CONTROL 0x3e diff --git a/src/util/storage_file.c b/src/util/storage_file.c index ede79fa..c427054 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -101,7 +101,9 @@ qedGetBackingStore(char **, int *, const unsigned char *, size_t); #define QCOW1_HDR_CRYPT (QCOWX_HDR_IMAGE_SIZE+8+1+1) #define QCOW2_HDR_CRYPT (QCOWX_HDR_IMAGE_SIZE+8) -#define QCOW1_HDR_TOTAL_SIZE (QCOW1_HDR_CRYPT+4+8) +#if 0 +# define QCOW1_HDR_TOTAL_SIZE (QCOW1_HDR_CRYPT+4+8) +#endif #define QCOW2_HDR_TOTAL_SIZE (QCOW2_HDR_CRYPT+4+4+8+8+4+4+8) #define QCOW2_HDR_EXTENSION_END 0 diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c index 9526ee4..6e680ba 100644 --- a/src/vbox/vbox_driver.c +++ b/src/vbox/vbox_driver.c @@ -42,8 +42,6 @@ #include "virterror_internal.h" #include "util.h" -#define VIR_FROM_THIS VIR_FROM_VBOX - extern virDriver vbox22Driver; extern virNetworkDriver vbox22NetworkDriver; @@ -66,7 +64,7 @@ static virDriver vboxDriverDummy; #define VIR_FROM_THIS VIR_FROM_VBOX #define vboxError(code, ...) \ - virReportErrorHelper(NULL, VIR_FROM_VBOX, code, __FILE__, \ + virReportErrorHelper(NULL, VIR_FROM_THIS, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) int vboxRegister(void) { diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 8a9dae5..d7d74a4 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -80,16 +80,12 @@ typedef struct v0_hypercall_struct { } v0_hypercall_t; #ifdef __linux__ -# define XEN_V0_IOCTL_HYPERCALL_CMD \ - _IOC(_IOC_NONE, 'P', 0, sizeof(v0_hypercall_t)) /* the new one */ typedef struct v1_hypercall_struct { uint64_t op; uint64_t arg[5]; } v1_hypercall_t; -# define XEN_V1_IOCTL_HYPERCALL_CMD \ - _IOC(_IOC_NONE, 'P', 0, sizeof(v1_hypercall_t)) typedef v1_hypercall_t hypercall_t; #elif defined(__sun) typedef privcmd_hypercall_t hypercall_t; @@ -140,8 +136,6 @@ static regex_t xen_cap_rec; # define DOMFLAGS_PAUSED (1<<3) /* Currently paused by control software. */ # define DOMFLAGS_BLOCKED (1<<4) /* Currently blocked pending an event. */ # define DOMFLAGS_RUNNING (1<<5) /* Domain is currently running. */ -# define DOMFLAGS_CPUMASK 255 /* CPU to which this domain is bound. */ -# define DOMFLAGS_CPUSHIFT 8 # define DOMFLAGS_SHUTDOWNMASK 255 /* DOMFLAGS_SHUTDOWN guest-supplied code. */ # define DOMFLAGS_SHUTDOWNSHIFT 16 #endif @@ -2709,13 +2703,13 @@ xenHypervisorMakeCapabilities(virConnectPtr conn) } } - capabilities = fopen ("/sys/hypervisor/properties/capabilities", "r"); + capabilities = fopen(HYPERVISOR_CAPABILITIES, "r"); if (capabilities == NULL) { if (errno != ENOENT) { VIR_FORCE_FCLOSE(cpuinfo); virReportSystemError(errno, _("cannot read file %s"), - "/sys/hypervisor/properties/capabilities"); + HYPERVISOR_CAPABILITIES); return NULL; } } @@ -3175,7 +3169,9 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) break; case DOMFLAGS_SHUTDOWN: /* The domain is shutdown. Determine the cause. */ - domain_shutdown_cause = domain_flags >> DOMFLAGS_SHUTDOWNSHIFT; + domain_shutdown_cause = + (domain_flags >> DOMFLAGS_SHUTDOWNSHIFT) & + DOMFLAGS_SHUTDOWNMASK; switch (domain_shutdown_cause) { case SHUTDOWN_crash: info->state = VIR_DOMAIN_CRASHED; diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 7f73588..6c43811 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -52,14 +52,6 @@ #define VIR_FROM_THIS VIR_FROM_XENXM -#ifdef WITH_RHEL5_API -# define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 0 -# define XEND_CONFIG_MIN_VERS_PVFB_NEWCONF 2 -#else -# define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 3 -# define XEND_CONFIG_MIN_VERS_PVFB_NEWCONF 3 -#endif - /* The true Xen limit varies but so far is always way less than 1024, which is the Linux kernel limit according to sched.h, so we'll match that for now */ @@ -78,7 +70,6 @@ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, #define XEND_CONFIG_FILE "xend-config.sxp" #define XEND_PCI_CONFIG_PREFIX "xend-pci-" #define QEMU_IF_SCRIPT "qemu-ifup" -#define XM_XML_ERROR "Invalid xml" struct xenUnifiedDriver xenXMDriver = { xenXMOpen, /* open */ diff --git a/tests/sockettest.c b/tests/sockettest.c index 2c9ff03..bb44496 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -170,16 +170,6 @@ mymain(int argc ATTRIBUTE_UNUSED, if (!virTestGetDebug()) virSetErrorFunc(NULL, testQuietError); -#define DO_TEST_PARSE(addrstr, family, pass) \ - do { \ - virSocketAddr addr; \ - struct testParseData data = { &addr, addrstr, family, pass }; \ - memset(&addr, 0, sizeof(addr)); \ - if (virtTestRun("Test parse " addrstr, \ - 1, testParseHelper, &data) < 0) \ - ret = -1; \ - } while (0) - #define DO_TEST_PARSE_AND_FORMAT(addrstr, family, pass) \ do { \ virSocketAddr addr; \ diff --git a/tools/virsh.c b/tools/virsh.c index 4765f5c..d1c3fe3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -80,12 +80,12 @@ static char *progname; */ #define MSG_BUFFER 4096 #define SIGN_NAME "virsh" -#define DIR_MODE (S_IWUSR | S_IRUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH) /* 0755 */ #define FILE_MODE (S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH) /* 0644 */ -#define LOCK_MODE (S_IWUSR | S_IRUSR) /* 0600 */ #define LVL_DEBUG "DEBUG" #define LVL_INFO "INFO" +#if 0 #define LVL_NOTICE "NOTICE" +#endif #define LVL_WARNING "WARNING" #define LVL_ERROR "ERROR" @@ -409,15 +409,6 @@ _vshStrdup(vshControl *ctl, const char *s, const char *filename, int line) exit(EXIT_FAILURE); } -/* Poison the raw allocating identifiers in favor of our vsh variants. */ -#undef malloc -#undef calloc -#undef realloc -#undef strdup -#define malloc use_vshMalloc_instead_of_malloc -#define calloc use_vshCalloc_instead_of_calloc -#define realloc use_vshRealloc_instead_of_realloc -#define strdup use_vshStrdup_instead_of_strdup static int idsorter(const void *a, const void *b) { const int *ia = (const int *)a; -- 1.7.4
On 04/04/2011 10:20 AM, Daniel P. Berrange wrote:
* m4/virt-compile-warnings.m4: Enable -Wunused-macros * daemon/libvirtd.c: Remove MAX_LISTEN * daemon/remote.c: Remote VIR_FROM_THIS
s/Remote/Remove/
33 files changed, 38 insertions(+), 108 deletions(-)
Nice ratio!
+++ b/src/esx/esx_vi_methods.c @@ -1,4 +1,3 @@ - /* * esx_vi_methods.c: client for the VMware VI API 2.5 to manage ESX hosts * @@ -52,8 +51,10 @@
+#if 0 #define ESX_VI__METHOD__CHECK_OUTPUT__RequiredList \
Won't this cause 'make syntax-check' grief during cppi checks? (Multiple instances)
+++ b/tools/virsh.c @@ -409,15 +409,6 @@ _vshStrdup(vshControl *ctl, const char *s, const char *filename, int line) exit(EXIT_FAILURE); }
-/* Poison the raw allocating identifiers in favor of our vsh variants. */ -#undef malloc -#undef calloc -#undef realloc -#undef strdup -#define malloc use_vshMalloc_instead_of_malloc -#define calloc use_vshCalloc_instead_of_calloc -#define realloc use_vshRealloc_instead_of_realloc -#define strdup use_vshStrdup_instead_of_strdup
Hmm, I don't want to completely lose this. Can we instead include "warn-on-use.h" (already provided by gnulib) and do: _GL_WARN_ON_USE(malloc, "use vshMalloc instead of malloc"); to get the same poisoning effects but via magic attributes rather than unused macros? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Apr 04, 2011 at 02:54:28PM -0600, Eric Blake wrote:
On 04/04/2011 10:20 AM, Daniel P. Berrange wrote:
* m4/virt-compile-warnings.m4: Enable -Wunused-macros * daemon/libvirtd.c: Remove MAX_LISTEN * daemon/remote.c: Remote VIR_FROM_THIS
s/Remote/Remove/
33 files changed, 38 insertions(+), 108 deletions(-)
Nice ratio!
+++ b/src/esx/esx_vi_methods.c @@ -1,4 +1,3 @@ - /* * esx_vi_methods.c: client for the VMware VI API 2.5 to manage ESX hosts * @@ -52,8 +51,10 @@
+#if 0 #define ESX_VI__METHOD__CHECK_OUTPUT__RequiredList \
Won't this cause 'make syntax-check' grief during cppi checks? (Multiple instances)
+++ b/tools/virsh.c @@ -409,15 +409,6 @@ _vshStrdup(vshControl *ctl, const char *s, const char *filename, int line) exit(EXIT_FAILURE); }
-/* Poison the raw allocating identifiers in favor of our vsh variants. */ -#undef malloc -#undef calloc -#undef realloc -#undef strdup -#define malloc use_vshMalloc_instead_of_malloc -#define calloc use_vshCalloc_instead_of_calloc -#define realloc use_vshRealloc_instead_of_realloc -#define strdup use_vshStrdup_instead_of_strdup
Hmm, I don't want to completely lose this. Can we instead include "warn-on-use.h" (already provided by gnulib) and do:
_GL_WARN_ON_USE(malloc, "use vshMalloc instead of malloc");
to get the same poisoning effects but via magic attributes rather than unused macros?
That doesn't work either: cc1: warnings being treated as errors virsh.c:366:1: error: redundant redeclaration of 'malloc' [-Wredundant-decls] virsh.c:367:1: error: redundant redeclaration of 'calloc' [-Wredundant-decls] virsh.c:368:1: error: redundant redeclaration of 'realloc' [-Wredundant-decls] virsh.c:369:1: error: redundant redeclaration of 'strdup' [-Wredundant-decls] What we really want is a syntax-check rule for these i reckon Regards, 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 :|
participants (2)
-
Daniel P. Berrange -
Eric Blake