Re: [virt-tools-list] [virt-viewer PATCH v2 3/3] Switch to gnulib's compiler warning flags

[Christophe Fergeau]

On Tue, Feb 19, 2019 at 03:00:11PM +, Daniel P. Berrangé wrote:
> diff --git a/m4/virt-viewer-warnings.m4 b/m4/virt-viewer-warnings.m4
> new file mode 100644
> index 000..4036b75
> --- /dev/null
> +++ b/m4/virt-viewer-warnings.m4
> [...]
> +# List of warnings that are not relevant / wanted
> +
> +# Don't care about C++ compiler compat
> +dontwarn="$dontwarn -Wc++-compat"
> +dontwarn="$dontwarn -Wabi"
> +dontwarn="$dontwarn -Wdeprecated"
> +# Don't care about ancient C standard compat
> +dontwarn="$dontwarn -Wtraditional"
> +# Don't care about ancient C standard compat
> +dontwarn="$dontwarn -Wtraditional-conversion"
> +# Ignore warnings in /usr/include
> +dontwarn="$dontwarn -Wsystem-headers"
> +# Happy for compiler to add struct padding
> +dontwarn="$dontwarn -Wpadded"
> +# GCC very confused with -O2
> +dontwarn="$dontwarn -Wunreachable-code"
> +# Too many to deal with
> +dontwarn="$dontwarn -Wconversion"
> +# Too many to deal with
> +dontwarn="$dontwarn -Wsign-conversion"
> +# We need to use long long in many places
> +dontwarn="$dontwarn -Wlong-long"
> +# Not a problem since we don't use -fstrict-overflow
> +dontwarn="$dontwarn -Wstrict-overflow"
> +# Not a problem since we don't use -funsafe-loop-optimizations
> +dontwarn="$dontwarn -Wunsafe-loop-optimizations"
> +# gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall
> +dontwarn="$dontwarn -Wenum-compare"
> +# /usr/include/spice-1/spice/protocol.h triggers violations
> +dontwarn="$dontwarn -Wpacked"
> +
> +# g_clear_object & G_ATOMIC_OP_USE_GCC_BUILTINS causes
> +# violations with this. XXX Fix glib ?
> +dontwarn="$dontwarn -Wbad-function-cast"
> +
> +# Due to gutils.h bug in g_bit_storage
> +wantwarn="$wantwarn -Wno-sign-conversion"
> +wantwarn="$wantwarn -Wno-conversion"
> +# We can't enable this due to horrible spice_usb_device_get_description
> +# signature
> +#wantwarn="$wantwarn -Wno-format-nonliteral"

I'd remove this, spice_usb_device_get_description is not marked as using
a format string, and uncommenting this line/changing it to
-Wformat-nonliteral did not trigger any additional warnings.

> +
> +# 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])
> +
> +# GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff.
> +# Unfortunately, this means you can't simply use '-Wsign-compare'
> +# with gl_MANYWARN_COMPLEMENT
> +# So we have -W enabled, and then have to explicitly turn off...
> +wantwarn="$wantwarn -Wno-sign-compare"
> +
> +# GNULIB expects this to be part of -Wc++-compat, but we turn
> +# that one off, so we need to manually enable this again
> +wantwarn="$wantwarn -Wjump-misses-init"
> +
> +# We do "bad" function casts all the time for event callbacks
> +wantwarn="$wantwarn -Wno-cast-function-type"
> +
> +# GNULIB turns on -Wformat=2 which implies -Wformat-nonliteral,
> +# so we need to manually re-exclude it.
> +wantwarn="$wantwarn -Wno-format-nonliteral"

This on can be removed too if we enable -Wformat-nonliteral.


signature.asc
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

[virt-tools-list] [virt-viewer PATCH v2 3/3] Switch to gnulib's compiler warning flags

[Daniel P . Berrangé]

This enables many more compiler warnings than the current code. It also
ensures that -Werror is enabled by default when building from GIT so
that maintainers see regressions as hard failures.

Reviewed-by: Marc-André Lureau 
Signed-off-by: Daniel P. Berrangé 
---
 acinclude.m4   |  98 ---
 configure.ac   |   2 +-
 m4/manywarnings.m4 | 334 +
 m4/virt-viewer-warnings.m4 | 159 ++
 m4/warnings.m4 | 115 +
 prepare-release.sh |   2 +-
 6 files changed, 610 insertions(+), 100 deletions(-)
 delete mode 100644 acinclude.m4
 create mode 100644 m4/manywarnings.m4
 create mode 100644 m4/virt-viewer-warnings.m4
 create mode 100644 m4/warnings.m4

diff --git a/acinclude.m4 b/acinclude.m4
deleted file mode 100644
index 68398a6..000
--- a/acinclude.m4
+++ /dev/null
@@ -1,98 +0,0 @@
-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
-AC_DEFUN([VIRT_VIEWER_COMPILE_WARNINGS],[
-dnl **
-dnl More compiler warnings
-dnl **
-
-AC_ARG_ENABLE(compile-warnings,
-  
AC_HELP_STRING([--enable-compile-warnings=@<:@no/minimum/yes/maximum/error@:>@],
- [Turn on compiler warnings]),,
-  [enable_compile_warnings="m4_default([$1],[maximum])"])
-
-warnCFLAGS=
-
-try_compiler_flags="-fexceptions -fstack-protector 
--param=ssp-buffer-size=4 -fasynchronous-unwind-tables"
-
-case "$enable_compile_warnings" in
-no)
-   ;;
-minimum)
-   try_compiler_flags="$try_compiler_flags -Wall"
-   ;;
-yes)
-   try_compiler_flags="$try_compiler_flags -Wall -Wmissing-prototypes 
-std=c99"
-   ;;
-maximum|error)
-   try_compiler_flags="$try_compiler_flags -Wall -Wmissing-prototypes 
-std=c99 -Wnested-externs -Wpointer-arith"
-try_compiler_flags="$try_compiler_flags -Wextra -Wshadow -Wcast-align 
-Wwrite-strings -Waggregate-return"
-# Removed -Wstrict-prototypes to avoid GTK bug
-   try_compiler_flags="$try_compiler_flags -Winline -Wredundant-decls 
-Wdeprecated-declarations -Wno-sign-compare"
-   # Remove as glib function casts hit
-   try_compiler_flags="$try_compiler_flags -Wno-cast-function-type"
-   if test "$enable_compile_warnings" = "error" ; then
-   try_compiler_flags="$try_compiler_flags -Werror"
-   fi
-   ;;
-*)
-   AC_MSG_ERROR(Unknown argument '$enable_compile_warnings' to 
--enable-compile-warnings)
-   ;;
-esac
-
-AH_VERBATIM([FORTIFY_SOURCE],
-[/* Enable compile-time and run-time bounds-checking, and some 
warnings. */
- #if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && 
__OPTIMIZE__
- # define _FORTIFY_SOURCE 2
- #endif
-])
-
-compiler_flags=
-for option in $try_compiler_flags; do
-   SAVE_CFLAGS="$CFLAGS"
-   CFLAGS="$CFLAGS $option"
-   AC_MSG_CHECKING([whether gcc understands $option])
-   AC_TRY_COMPILE([], [],
-   has_option=yes,
-   has_option=no,)
-   CFLAGS="$SAVE_CFLAGS"
-   AC_MSG_RESULT($has_option)
-   if test $has_option = yes; then
- compiler_flags="$compiler_flags $option"
-   fi
-   unset has_option
-   unset SAVE_CFLAGS
-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"
-AC_SUBST(WARN_CFLAGS)
-])
diff --git a/configure.ac b/configure.ac
index 5598c61..8ff69ac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -90,7 +90,7 @@ m4_if(m4_version_compare([2.61a.100],
   [AC_CONFIG_LINKS([$GNUmakefile:$GNUmakefile], [],
 [GNUmakefile=$GNUmakefile])])
 
-VIRT_VIEWER_COMPILE_WARNINGS(maximum)
+VIRT_VIEWER_COMPILE_WARNINGS()
 
 GETTEXT_PACKAGE=virt-viewer
 AC_SUBST(GETTEXT_PACKAGE)
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
new file mode 100644
index