Be more careful choosing warning flags

7 files changed, 224 insertions, 44 deletions

diff --git a/client/Makefile.am b/client/Makefile.am
index efebfee..a363d90 100644
--- a/client/Makefile.am
+++ b/client/Makefile.am
@@ -40,7 +40,7 @@ appstream_util_LDADD =					\
 appstream_util_LDFLAGS =				\
 	$(PIE_LDFLAGS)
 appstream_util_CFLAGS =					\
-	$(WARNINGFLAGS_C)
+	$(WARN_CFLAGS)
 
 if HAVE_BUILDER
 appstream_builder_SOURCES =				\
@@ -55,7 +55,7 @@ appstream_builder_LDADD =				\
 appstream_builder_LDFLAGS =				\
 	$(PIE_LDFLAGS)
 appstream_builder_CFLAGS =				\
-	$(WARNINGFLAGS_C)
+	$(WARN_CFLAGS)
 endif
 
 appstream_compose_SOURCES =				\
@@ -69,6 +69,6 @@ appstream_compose_LDADD =				\
 appstream_compose_LDFLAGS =				\
 	$(PIE_LDFLAGS)
 appstream_compose_CFLAGS =				\
-	$(WARNINGFLAGS_C)
+	$(WARN_CFLAGS)
 
 -include $(top_srcdir)/git.mk
diff --git a/configure.ac b/configure.ac
index ad13156..bad7425 100644
--- a/configure.ac
+++ b/configure.ac
@@ -65,6 +65,31 @@ AC_HEADER_STDC
 LT_INIT
 AM_PROG_CC_C_O
 
+GNOME_COMPILE_WARNINGS([maximum],[
+	-Wmissing-declarations
+	-Wcast-align
+	-Wwrite-strings
+	-Wreturn-type
+	-Wformat-nonliteral
+	-Wmissing-format-attribute
+	-Wclobbered
+	-Wempty-body
+	-Wignored-qualifiers
+	-Wsign-compare
+	-Wtype-limits
+	-Wuninitialized
+	-Waggregate-return
+	-Wdeclaration-after-statement
+	-Wshadow
+	-Wno-strict-aliasing
+	-Winline
+	-Wmissing-parameter-type
+	-Woverride-init
+	-Wno-discarded-qualifiers
+	-Wconversion
+	-Wformat-signedness
+])
+
 # internationalization
 GETTEXT_PACKAGE=appstream-glib
 AC_SUBST(GETTEXT_PACKAGE)
@@ -74,35 +99,6 @@ IT_PROG_INTLTOOL([0.40.0])
 # set up gtk-doc
 GTK_DOC_CHECK(1.9)
 
-if test "$GCC" = "yes"; then
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wall"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wcast-align -Wno-uninitialized"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wmissing-declarations"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wredundant-decls"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wpointer-arith"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wcast-align"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wwrite-strings"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Winit-self"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wreturn-type"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wformat-nonliteral"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wformat-security"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wmissing-include-dirs"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wmissing-format-attribute"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wsign-compare"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wtype-limits"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wuninitialized"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Waggregate-return"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wdeclaration-after-statement"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wshadow"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wno-strict-aliasing"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Winline"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wconversion"
-	WARNINGFLAGS_C="$WARNINGFLAGS_C -Wformat-signedness"
-else
-	WARNINGFLAGS_C=""
-fi
-AC_SUBST(WARNINGFLAGS_C)
-
 # check for PIE (position independent executable) support
 AX_CHECK_COMPILE_FLAG([-fPIE],
 		      [AX_CHECK_LINK_FLAG([-fPIE -pie],
diff --git a/libappstream-builder/Makefile.am b/libappstream-builder/Makefile.am
index bbc7569..4e86023 100644
--- a/libappstream-builder/Makefile.am
+++ b/libappstream-builder/Makefile.am
@@ -90,7 +90,7 @@ libappstream_builder_la_LDFLAGS =				\
 	-export-symbols-regex '^asb_.*'
 
 libappstream_builder_la_CFLAGS =				\
-	$(WARNINGFLAGS_C)
+	$(WARN_CFLAGS)
 
 check_PROGRAMS =						\
 	asb-self-test
@@ -101,7 +101,7 @@ asb_self_test_LDADD =						\
 	$(GLIB_LIBS)						\
 	$(GDKPIXBUF_LIBS)					\
 	$(lib_LTLIBRARIES)
-asb_self_test_CFLAGS = $(WARNINGFLAGS_C)
+asb_self_test_CFLAGS = $(WARN_CFLAGS)
 
 TESTS = asb-self-test
 
diff --git a/libappstream-builder/asb-utils.c b/libappstream-builder/asb-utils.c
index c5105d8..01b4f04 100644
--- a/libappstream-builder/asb-utils.c
+++ b/libappstream-builder/asb-utils.c
@@ -169,7 +169,7 @@ asb_utils_ensure_exists_and_empty (const gchar *directory, GError **error)
 	return TRUE;
 }
 
-static const guint
+static guint
 asb_utils_count_directories_deep (const gchar *path)
 {
 	guint cnt = 0;
diff --git a/libappstream-builder/plugins/Makefile.am b/libappstream-builder/plugins/Makefile.am
index 861fa37..ec82301 100644
--- a/libappstream-builder/plugins/Makefile.am
+++ b/libappstream-builder/plugins/Makefile.am
@@ -30,38 +30,38 @@ endif
 libasb_plugin_absorb_la_SOURCES = asb-plugin-absorb.c
 libasb_plugin_absorb_la_LIBADD = $(GLIB_LIBS)
 libasb_plugin_absorb_la_LDFLAGS = -module -avoid-version
-libasb_plugin_absorb_la_CFLAGS = $(GLIB_CFLAGS) $(WARNINGFLAGS_C)
+libasb_plugin_absorb_la_CFLAGS = $(GLIB_CFLAGS) $(WARN_CFLAGS)
 
 libasb_plugin_gettext_la_SOURCES = asb-plugin-gettext.c
 libasb_plugin_gettext_la_LIBADD = $(GLIB_LIBS)
 libasb_plugin_gettext_la_LDFLAGS = -module -avoid-version
-libasb_plugin_gettext_la_CFLAGS = $(GLIB_CFLAGS) $(WARNINGFLAGS_C)
+libasb_plugin_gettext_la_CFLAGS = $(GLIB_CFLAGS) $(WARN_CFLAGS)
 
 libasb_plugin_hardcoded_la_SOURCES = asb-plugin-hardcoded.c
 libasb_plugin_hardcoded_la_LIBADD = $(GLIB_LIBS)
 libasb_plugin_hardcoded_la_LDFLAGS = -module -avoid-version
-libasb_plugin_hardcoded_la_CFLAGS = $(GLIB_CFLAGS) $(WARNINGFLAGS_C)
+libasb_plugin_hardcoded_la_CFLAGS = $(GLIB_CFLAGS) $(WARN_CFLAGS)
 
 libasb_plugin_desktop_la_SOURCES = asb-plugin-desktop.c
 libasb_plugin_desktop_la_LIBADD = $(GLIB_LIBS) $(GDKPIXBUF_LIBS)
 libasb_plugin_desktop_la_LDFLAGS = -module -avoid-version
-libasb_plugin_desktop_la_CFLAGS = $(GLIB_CFLAGS) $(WARNINGFLAGS_C)
+libasb_plugin_desktop_la_CFLAGS = $(GLIB_CFLAGS) $(WARN_CFLAGS)
 
 libasb_plugin_appdata_la_SOURCES = asb-plugin-appdata.c
 libasb_plugin_appdata_la_LIBADD = $(GLIB_LIBS) $(GDKPIXBUF_LIBS)
 libasb_plugin_appdata_la_LDFLAGS = -module -avoid-version
-libasb_plugin_appdata_la_CFLAGS = $(GLIB_CFLAGS) $(WARNINGFLAGS_C)
+libasb_plugin_appdata_la_CFLAGS = $(GLIB_CFLAGS) $(WARN_CFLAGS)
 
 libasb_plugin_shell_extension_la_SOURCES = asb-plugin-shell-extension.c
 libasb_plugin_shell_extension_la_LIBADD = $(GLIB_LIBS) $(JSON_GLIB_LIBS)
 libasb_plugin_shell_extension_la_LDFLAGS = -module -avoid-version
-libasb_plugin_shell_extension_la_CFLAGS = $(GLIB_CFLAGS) $(WARNINGFLAGS_C)
+libasb_plugin_shell_extension_la_CFLAGS = $(GLIB_CFLAGS) $(WARN_CFLAGS)
 
 if HAVE_FONTS
 libasb_plugin_font_la_SOURCES = asb-plugin-font.c
 libasb_plugin_font_la_LIBADD = $(GLIB_LIBS) $(FREETYPE_LIBS) $(GDKPIXBUF_LIBS) $(GTK_LIBS)
 libasb_plugin_font_la_LDFLAGS = -module -avoid-version
-libasb_plugin_font_la_CFLAGS = $(GLIB_CFLAGS) $(GTK_CFLAGS) $(WARNINGFLAGS_C)
+libasb_plugin_font_la_CFLAGS = $(GLIB_CFLAGS) $(GTK_CFLAGS) $(WARN_CFLAGS)
 endif
 
 -include $(top_srcdir)/git.mk
diff --git a/libappstream-glib/Makefile.am b/libappstream-glib/Makefile.am
index 0aaeee7..f58b826 100644
--- a/libappstream-glib/Makefile.am
+++ b/libappstream-glib/Makefile.am
@@ -169,7 +169,7 @@ libappstream_glib_la_LDFLAGS =					\
 	-export-symbols-regex '^as_.*'
 
 libappstream_glib_la_CFLAGS =					\
-	$(WARNINGFLAGS_C)
+	$(WARN_CFLAGS)
 
 check_PROGRAMS =						\
 	as-self-test
@@ -184,7 +184,7 @@ as_self_test_LDADD =						\
 	$(UUID_LIBS)						\
 	$(YAML_LIBS)						\
 	$(lib_LTLIBRARIES)
-as_self_test_CFLAGS = $(WARNINGFLAGS_C)
+as_self_test_CFLAGS = $(WARN_CFLAGS)
 
 TESTS = as-self-test
 
diff --git a/m4/gnome-compiler-flags.m4 b/m4/gnome-compiler-flags.m4
new file mode 100644
index 0000000..49ebb95
--- /dev/null
+++ b/m4/gnome-compiler-flags.m4
@@ -0,0 +1,184 @@
+# gnome-compiler-flags.m4
+#
+# serial 4
+#
+
+dnl GNOME_COMPILE_WARNINGS
+dnl Turn on many useful compiler warnings and substitute the result into
+dnl WARN_CFLAGS
+dnl For now, only works on GCC
+dnl Pass the default value of the --enable-compile-warnings configure option as
+dnl the first argument to the macro, defaulting to 'yes'.
+dnl Additional warning/error flags can be passed as an optional second argument.
+dnl
+dnl For example: GNOME_COMPILE_WARNINGS([maximum],[-Werror=some-flag -Wfoobar])
+AU_DEFUN([GNOME_COMPILE_WARNINGS],[
+    dnl ******************************
+    dnl More compiler warnings
+    dnl ******************************
+
+    AC_ARG_ENABLE(compile-warnings, 
+                  AS_HELP_STRING([--enable-compile-warnings=@<:@no/minimum/yes/maximum/error@:>@],
+                                 [Turn on compiler warnings]),,
+                  [enable_compile_warnings="m4_default([$1],[yes])"])
+
+    if test "x$GCC" != xyes; then
+	enable_compile_warnings=no
+    fi
+
+    warning_flags=
+    realsave_CFLAGS="$CFLAGS"
+
+    dnl These are warning flags that aren't marked as fatal.  Can be
+    dnl overridden on a per-project basis with -Wno-foo.
+    base_warn_flags=" \
+        -Wall \
+        -Wstrict-prototypes \
+        -Wnested-externs \
+    "
+
+    dnl These compiler flags typically indicate very broken or suspicious
+    dnl code.  Some of them such as implicit-function-declaration are
+    dnl just not default because gcc compiles a lot of legacy code.
+    dnl We choose to make this set into explicit errors.
+    base_error_flags=" \
+        -Werror=missing-prototypes \
+        -Werror=implicit-function-declaration \
+        -Werror=pointer-arith \
+        -Werror=init-self \
+        -Werror=format-security \
+        -Werror=format=2 \
+        -Werror=missing-include-dirs \
+        -Werror=return-type \
+    "
+
+    dnl Additional warning or error flags provided by the module author to
+    dnl allow stricter standards to be imposed on a per-module basis.
+    dnl The author can pass -W or -Werror flags here as they see fit.
+    additional_flags="m4_default([$2],[])"
+
+    case "$enable_compile_warnings" in
+    no)
+        warning_flags="-w"
+        ;;
+    minimum)
+        warning_flags="-Wall"
+        ;;
+    yes|maximum|error)
+        warning_flags="$base_warn_flags $base_error_flags $additional_flags"
+        ;;
+    *)
+        AC_MSG_ERROR(Unknown argument '$enable_compile_warnings' to --enable-compile-warnings)
+        ;;
+    esac
+
+    if test "$enable_compile_warnings" = "error" ; then
+        warning_flags="$warning_flags -Werror"
+    fi
+
+    dnl Check whether GCC supports the warning options
+    for option in $warning_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
+	    tested_warning_flags="$tested_warning_flags $option"
+	fi
+	unset has_option
+	unset save_CFLAGS
+    done
+    unset option
+    CFLAGS="$realsave_CFLAGS"
+    AC_MSG_CHECKING(what warning flags to pass to the C compiler)
+    AC_MSG_RESULT($tested_warning_flags)
+
+    AC_ARG_ENABLE(iso-c,
+                  AS_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="$tested_warning_flags $complCFLAGS"
+    AC_SUBST(WARN_CFLAGS)
+],
+[[$0: This macro is deprecated. You should use AX_COMPILER_FLAGS instead and
+eliminate use of --enable-iso-c.
+See: http://www.gnu.org/software/autoconf-archive/ax_compiler_flags.html#ax_compiler_flags]])
+
+dnl For C++, do basically the same thing.
+
+AU_DEFUN([GNOME_CXX_WARNINGS],[
+  AC_ARG_ENABLE(cxx-warnings,
+                AS_HELP_STRING([--enable-cxx-warnings=@<:@no/minimum/yes@:>@]
+                               [Turn on compiler warnings.]),,
+                [enable_cxx_warnings="m4_default([$1],[minimum])"])
+
+  AC_MSG_CHECKING(what warning flags to pass to the C++ compiler)
+  warnCXXFLAGS=
+  if test "x$GXX" != xyes; then
+    enable_cxx_warnings=no
+  fi
+  if test "x$enable_cxx_warnings" != "xno"; then
+    if test "x$GXX" = "xyes"; then
+      case " $CXXFLAGS " in
+      *[\ \	]-Wall[\ \	]*) ;;
+      *) warnCXXFLAGS="-Wall -Wno-unused" ;;
+      esac
+
+      ## -W is not all that useful.  And it cannot be controlled
+      ## with individual -Wno-xxx flags, unlike -Wall
+      if test "x$enable_cxx_warnings" = "xyes"; then
+	warnCXXFLAGS="$warnCXXFLAGS -Wshadow -Woverloaded-virtual"
+      fi
+    fi
+  fi
+  AC_MSG_RESULT($warnCXXFLAGS)
+
+   AC_ARG_ENABLE(iso-cxx,
+                 AS_HELP_STRING([--enable-iso-cxx],
+                                [Try to warn if code is not ISO C++ ]),,
+                 [enable_iso_cxx=no])
+
+   AC_MSG_CHECKING(what language compliance flags to pass to the C++ compiler)
+   complCXXFLAGS=
+   if test "x$enable_iso_cxx" != "xno"; then
+     if test "x$GXX" = "xyes"; then
+      case " $CXXFLAGS " in
+      *[\ \	]-ansi[\ \	]*) ;;
+      *) complCXXFLAGS="$complCXXFLAGS -ansi" ;;
+      esac
+
+      case " $CXXFLAGS " in
+      *[\ \	]-pedantic[\ \	]*) ;;
+      *) complCXXFLAGS="$complCXXFLAGS -pedantic" ;;
+      esac
+     fi
+   fi
+  AC_MSG_RESULT($complCXXFLAGS)
+
+  WARN_CXXFLAGS="$CXXFLAGS $warnCXXFLAGS $complCXXFLAGS"
+  AC_SUBST(WARN_CXXFLAGS)
+],
+[[$0: This macro is deprecated. You should use AX_COMPILER_FLAGS instead and
+eliminate use of --enable-iso-cxx.
+See: http://www.gnu.org/software/autoconf-archive/ax_compiler_flags.html#ax_compiler_flags]])