From 2618a19716856d2b107acc0b5b63838e1379ef45 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 22 Mar 2017 14:50:48 -0400 Subject: [PATCH] Import ostree's compiler warnings, fix up callers In ostree I maintain what I consider a "baseline" set of compiler warnings that should *always* be fatal for a modern C project. I noticed while working on a previous patch that a `-Werror=format` warning wasn't fatal. There are a few that are really, really important like `-Werror=missing-prototypes`. I also take some like `-Werror=misleading-indentation` which already caught some bugs. See also https://lwn.net/Articles/678019/ --- .gitignore | 1 + Makefile.am | 2 + common/flatpak-dir.c | 18 +-- common/flatpak-run.c | 8 +- common/flatpak-utils.c | 13 +- common/flatpak-utils.h | 6 +- configure.ac | 26 +++- lib/flatpak-installed-ref.c | 1 + lib/flatpak-related-ref.c | 1 + m4/attributes.m4 | 292 ++++++++++++++++++++++++++++++++++++ 10 files changed, 343 insertions(+), 25 deletions(-) create mode 100644 m4/attributes.m4 diff --git a/.gitignore b/.gitignore index 6e979cca..796332b4 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ libtool ltmain.sh m4/*.m4 !m4/glibtests.m4 +!m4/attributes.m4 missing stamp-h1 config.h.in diff --git a/Makefile.am b/Makefile.am index b1ec532e..2576d455 100644 --- a/Makefile.am +++ b/Makefile.am @@ -53,6 +53,8 @@ AM_CPPFLAGS = \ -I$(builddir)/lib \ $(NULL) +AM_CFLAGS = $(WARN_CFLAGS) + if WITH_SYSTEM_BWRAP AM_CPPFLAGS += -DHELPER=\"$(BWRAP)\" else diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c index 70d49725..0d792799 100644 --- a/common/flatpak-dir.c +++ b/common/flatpak-dir.c @@ -576,7 +576,7 @@ flatpak_get_user_base_dir_location (void) return g_object_ref ((GFile *)file); } -GFile * +static GFile * flatpak_get_user_cache_dir_location (void) { g_autoptr(GFile) base_dir = NULL; @@ -585,7 +585,7 @@ flatpak_get_user_cache_dir_location (void) return g_file_get_child (base_dir, "system-cache"); } -GFile * +static GFile * flatpak_ensure_user_cache_dir_location (GError **error) { g_autoptr(GFile) cache_dir = NULL; @@ -603,7 +603,7 @@ flatpak_ensure_user_cache_dir_location (GError **error) return g_steal_pointer (&cache_dir); } -GFile * +static GFile * flatpak_ensure_oci_summary_cache_dir_location (GError **error) { g_autoptr(GFile) cache_dir = NULL; @@ -658,7 +658,7 @@ flatpak_dir_get_system_helper (FlatpakDir *self) return NULL; } -gboolean +static gboolean flatpak_dir_use_system_helper (FlatpakDir *self, const char *installing_from_remote) { @@ -2921,7 +2921,7 @@ out: return ret; } -gboolean +static gboolean flatpak_dir_run_triggers (FlatpakDir *self, GCancellable *cancellable, GError **error) @@ -3349,7 +3349,7 @@ out: return ret; } -gboolean +static gboolean flatpak_rewrite_export_dir (const char *app, const char *branch, const char *arch, @@ -3467,7 +3467,7 @@ out: return ret; } -gboolean +static gboolean flatpak_export_dir (GFile *source, GFile *destination, const char *symlink_prefix, @@ -6146,7 +6146,7 @@ find_matching_ref (GHashTable *refs, parts[3])); } - flatpak_fail (error, err->str); + flatpak_fail (error, "%s", err->str); return NULL; } @@ -6606,7 +6606,7 @@ flatpak_dir_remote_has_deploys (FlatpakDir *self, return FALSE; } -gint +static gint cmp_remote (gconstpointer a, gconstpointer b, gpointer user_data) diff --git a/common/flatpak-run.c b/common/flatpak-run.c index 8b734f8a..3fb58a3c 100644 --- a/common/flatpak-run.c +++ b/common/flatpak-run.c @@ -497,7 +497,7 @@ flatpak_context_set_system_bus_policy (FlatpakContext *context, g_hash_table_insert (context->system_bus_policy, g_strdup (name), GINT_TO_POINTER (policy)); } -void +static void flatpak_context_apply_generic_policy (FlatpakContext *context, const char *key, const char *value) @@ -2075,7 +2075,7 @@ create_proxy_socket (char *template) return g_steal_pointer (&proxy_socket); } -gboolean +static gboolean flatpak_run_add_system_dbus_args (FlatpakContext *context, char ***envp_p, GPtrArray *argv_array, @@ -2127,7 +2127,7 @@ flatpak_run_add_system_dbus_args (FlatpakContext *context, return FALSE; } -gboolean +static gboolean flatpak_run_add_session_dbus_args (GPtrArray *argv_array, char ***envp_p, GPtrArray *dbus_proxy_argv, @@ -3487,7 +3487,7 @@ add_document_portal_args (GPtrArray *argv_array, } } -gchar * +static gchar * join_args (GPtrArray *argv_array, gsize *len_out) { gchar *string; diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c index c5f006bb..c6d122ea 100644 --- a/common/flatpak-utils.c +++ b/common/flatpak-utils.c @@ -285,7 +285,7 @@ flatpak_path_match_prefix (const char *pattern, return NULL; /* Should not be reached */ } -const char * +static const char * flatpak_get_kernel_arch (void) { static struct utsname buf; @@ -858,7 +858,7 @@ flatpak_kinds_from_bools (gboolean app, gboolean runtime) return kinds; } -gboolean +static gboolean _flatpak_split_partial_ref_arg (const char *partial_ref, gboolean validate, FlatpakKinds default_kinds, @@ -4695,9 +4695,13 @@ flatpak_yes_no_prompt (const char *prompt, ...) va_list var_args; gchar *s; + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-nonliteral" va_start (var_args, prompt); s = g_strdup_vprintf (prompt, var_args); va_end (var_args); +#pragma GCC diagnostic pop while (TRUE) { @@ -5098,9 +5102,12 @@ flatpak_complete_word (FlatpakCompletion *completion, g_return_if_fail (format != NULL); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-nonliteral" va_start (args, format); string = g_strdup_vprintf (format, args); va_end (args); +#pragma GCC diagnostic pop if (!g_str_has_prefix (string, completion->cur)) return; @@ -5144,7 +5151,7 @@ flatpak_complete_ref (FlatpakCompletion *completion, } } -int +static int find_current_element (const char *str) { int count = 0; diff --git a/common/flatpak-utils.h b/common/flatpak-utils.h index 96e42204..36ffb30b 100644 --- a/common/flatpak-utils.h +++ b/common/flatpak-utils.h @@ -579,8 +579,8 @@ gboolean flatpak_allocate_tmpdir (int tmpdir_dfd, GError **error); -gboolean flatpak_yes_no_prompt (const char *prompt, ...); -long flatpak_number_prompt (int min, int max, const char *prompt, ...); +gboolean flatpak_yes_no_prompt (const char *prompt, ...) G_GNUC_PRINTF(1, 2); +long flatpak_number_prompt (int min, int max, const char *prompt, ...) G_GNUC_PRINTF(3, 4); typedef void (*FlatpakLoadUriProgress) (guint64 downloaded_bytes, gpointer user_data); @@ -621,7 +621,7 @@ FlatpakCompletion *flatpak_completion_new (const char *arg_line, const char *arg_cur); void flatpak_complete_word (FlatpakCompletion *completion, char *format, - ...); + ...) G_GNUC_PRINTF(2,3); void flatpak_complete_ref (FlatpakCompletion *completion, OstreeRepo *repo); void flatpak_complete_partial_ref (FlatpakCompletion *completion, diff --git a/configure.ac b/configure.ac index c9c08448..fdd73fb2 100644 --- a/configure.ac +++ b/configure.ac @@ -55,12 +55,26 @@ AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE, "$PACKAGE", [gettext domain]) AM_SILENT_RULES([yes]) AM_MAINTAINER_MODE([enable]) -if test "x$GCC" = "xyes"; then - case " $CFLAGS " in - *[[\ \ ]]-Wall[[\ \ ]]*) ;; - *) CFLAGS="$CFLAGS -Wall" ;; - esac -fi +dnl This list is shared with https://github.com/ostreedev/ostree +CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ +-pipe \ +-Wall \ +-Werror=empty-body \ +-Werror=strict-prototypes \ +-Werror=missing-prototypes \ +-Werror=implicit-function-declaration \ +"-Werror=format=2 -Werror=format-security -Werror=format-nonliteral" \ +-Werror=pointer-arith -Werror=init-self \ +-Werror=missing-declarations \ +-Werror=return-type \ +-Werror=overflow \ +-Werror=int-conversion \ +-Werror=parenthesis \ +-Werror=incompatible-pointer-types \ +-Werror=misleading-indentation \ +-Werror=missing-include-dirs -Werror=aggregate-return \ +]) +AC_SUBST(WARN_CFLAGS) AX_VALGRIND_CHECK diff --git a/lib/flatpak-installed-ref.c b/lib/flatpak-installed-ref.c index ed830d66..6d1f3268 100644 --- a/lib/flatpak-installed-ref.c +++ b/lib/flatpak-installed-ref.c @@ -24,6 +24,7 @@ #include "flatpak-utils.h" #include "flatpak-installed-ref.h" +#include "flatpak-installed-ref-private.h" #include "flatpak-enum-types.h" /** diff --git a/lib/flatpak-related-ref.c b/lib/flatpak-related-ref.c index cf5216f2..133b4f66 100644 --- a/lib/flatpak-related-ref.c +++ b/lib/flatpak-related-ref.c @@ -24,6 +24,7 @@ #include "flatpak-utils.h" #include "flatpak-related-ref.h" +#include "flatpak-related-ref-private.h" #include "flatpak-enum-types.h" /** diff --git a/m4/attributes.m4 b/m4/attributes.m4 new file mode 100644 index 00000000..51ac88be --- /dev/null +++ b/m4/attributes.m4 @@ -0,0 +1,292 @@ +dnl Macros to check the presence of generic (non-typed) symbols. +dnl Copyright (c) 2006-2008 Diego Pettenò +dnl Copyright (c) 2006-2008 xine project +dnl Copyright (c) 2012 Lucas De Marchi +dnl +dnl This program is free software; you can redistribute it and/or modify +dnl it under the terms of the GNU General Public License as published by +dnl the Free Software Foundation; either version 2, or (at your option) +dnl any later version. +dnl +dnl This program is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +dnl GNU General Public License for more details. +dnl +dnl You should have received a copy of the GNU General Public License +dnl along with this program; if not, write to the Free Software +dnl Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +dnl 02110-1301, USA. +dnl +dnl As a special exception, the copyright owners of the +dnl macro gives unlimited permission to copy, distribute and modify the +dnl configure scripts that are the output of Autoconf when processing the +dnl Macro. You need not follow the terms of the GNU General Public +dnl License when using or distributing such scripts, even though portions +dnl of the text of the Macro appear in them. The GNU General Public +dnl License (GPL) does govern all other use of the material that +dnl constitutes the Autoconf Macro. +dnl +dnl This special exception to the GPL applies to versions of the +dnl Autoconf Macro released by this project. When you make and +dnl distribute a modified version of the Autoconf Macro, you may extend +dnl this special exception to the GPL to apply to your modified version as +dnl well. + +dnl Check if FLAG in ENV-VAR is supported by compiler and append it +dnl to WHERE-TO-APPEND variable. Note that we invert -Wno-* checks to +dnl -W* as gcc cannot test for negated warnings. If a C snippet is passed, +dnl use it, otherwise use a simple main() definition that just returns 0. +dnl CC_CHECK_FLAG_APPEND([WHERE-TO-APPEND], [ENV-VAR], [FLAG], [C-SNIPPET]) + +AC_DEFUN([CC_CHECK_FLAG_APPEND], [ + AC_CACHE_CHECK([if $CC supports flag $3 in envvar $2], + AS_TR_SH([cc_cv_$2_$3]), + [eval "AS_TR_SH([cc_save_$2])='${$2}'" + eval "AS_TR_SH([$2])='${cc_save_$2} -Werror `echo "$3" | sed 's/^-Wno-/-W/'`'" + AC_LINK_IFELSE([AC_LANG_SOURCE(ifelse([$4], [], + [int main(void) { return 0; } ], + [$4]))], + [eval "AS_TR_SH([cc_cv_$2_$3])='yes'"], + [eval "AS_TR_SH([cc_cv_$2_$3])='no'"]) + eval "AS_TR_SH([$2])='$cc_save_$2'"]) + + AS_IF([eval test x$]AS_TR_SH([cc_cv_$2_$3])[ = xyes], + [eval "$1='${$1} $3'"]) +]) + +dnl CC_CHECK_FLAGS_APPEND([WHERE-TO-APPEND], [ENV-VAR], [FLAG1 FLAG2], [C-SNIPPET]) +AC_DEFUN([CC_CHECK_FLAGS_APPEND], [ + for flag in [$3]; do + CC_CHECK_FLAG_APPEND([$1], [$2], $flag, [$4]) + done +]) + +dnl Check if the flag is supported by linker (cacheable) +dnl CC_CHECK_LDFLAGS([FLAG], [ACTION-IF-FOUND],[ACTION-IF-NOT-FOUND]) + +AC_DEFUN([CC_CHECK_LDFLAGS], [ + AC_CACHE_CHECK([if $CC supports $1 flag], + AS_TR_SH([cc_cv_ldflags_$1]), + [ac_save_LDFLAGS="$LDFLAGS" + LDFLAGS="$LDFLAGS $1" + AC_LINK_IFELSE([int main() { return 1; }], + [eval "AS_TR_SH([cc_cv_ldflags_$1])='yes'"], + [eval "AS_TR_SH([cc_cv_ldflags_$1])="]) + LDFLAGS="$ac_save_LDFLAGS" + ]) + + AS_IF([eval test x$]AS_TR_SH([cc_cv_ldflags_$1])[ = xyes], + [$2], [$3]) +]) + +dnl define the LDFLAGS_NOUNDEFINED variable with the correct value for +dnl the current linker to avoid undefined references in a shared object. +AC_DEFUN([CC_NOUNDEFINED], [ + dnl We check $host for which systems to enable this for. + AC_REQUIRE([AC_CANONICAL_HOST]) + + case $host in + dnl FreeBSD (et al.) does not complete linking for shared objects when pthreads + dnl are requested, as different implementations are present; to avoid problems + dnl use -Wl,-z,defs only for those platform not behaving this way. + *-freebsd* | *-openbsd*) ;; + *) + dnl First of all check for the --no-undefined variant of GNU ld. This allows + dnl for a much more readable command line, so that people can understand what + dnl it does without going to look for what the heck -z defs does. + for possible_flags in "-Wl,--no-undefined" "-Wl,-z,defs"; do + CC_CHECK_LDFLAGS([$possible_flags], [LDFLAGS_NOUNDEFINED="$possible_flags"]) + break + done + ;; + esac + + AC_SUBST([LDFLAGS_NOUNDEFINED]) +]) + +dnl Check for a -Werror flag or equivalent. -Werror is the GCC +dnl and ICC flag that tells the compiler to treat all the warnings +dnl as fatal. We usually need this option to make sure that some +dnl constructs (like attributes) are not simply ignored. +dnl +dnl Other compilers don't support -Werror per se, but they support +dnl an equivalent flag: +dnl - Sun Studio compiler supports -errwarn=%all +AC_DEFUN([CC_CHECK_WERROR], [ + AC_CACHE_CHECK( + [for $CC way to treat warnings as errors], + [cc_cv_werror], + [CC_CHECK_CFLAGS_SILENT([-Werror], [cc_cv_werror=-Werror], + [CC_CHECK_CFLAGS_SILENT([-errwarn=%all], [cc_cv_werror=-errwarn=%all])]) + ]) +]) + +AC_DEFUN([CC_CHECK_ATTRIBUTE], [ + AC_REQUIRE([CC_CHECK_WERROR]) + AC_CACHE_CHECK([if $CC supports __attribute__(( ifelse([$2], , [$1], [$2]) ))], + AS_TR_SH([cc_cv_attribute_$1]), + [ac_save_CFLAGS="$CFLAGS" + CFLAGS="$CFLAGS $cc_cv_werror" + AC_COMPILE_IFELSE([AC_LANG_SOURCE([$3])], + [eval "AS_TR_SH([cc_cv_attribute_$1])='yes'"], + [eval "AS_TR_SH([cc_cv_attribute_$1])='no'"]) + CFLAGS="$ac_save_CFLAGS" + ]) + + AS_IF([eval test x$]AS_TR_SH([cc_cv_attribute_$1])[ = xyes], + [AC_DEFINE( + AS_TR_CPP([SUPPORT_ATTRIBUTE_$1]), 1, + [Define this if the compiler supports __attribute__(( ifelse([$2], , [$1], [$2]) ))] + ) + $4], + [$5]) +]) + +AC_DEFUN([CC_ATTRIBUTE_CONSTRUCTOR], [ + CC_CHECK_ATTRIBUTE( + [constructor],, + [void __attribute__((constructor)) ctor() { int a; }], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_FORMAT], [ + CC_CHECK_ATTRIBUTE( + [format], [format(printf, n, n)], + [void __attribute__((format(printf, 1, 2))) printflike(const char *fmt, ...) { fmt = (void *)0; }], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_FORMAT_ARG], [ + CC_CHECK_ATTRIBUTE( + [format_arg], [format_arg(printf)], + [char *__attribute__((format_arg(1))) gettextlike(const char *fmt) { fmt = (void *)0; }], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_VISIBILITY], [ + CC_CHECK_ATTRIBUTE( + [visibility_$1], [visibility("$1")], + [void __attribute__((visibility("$1"))) $1_function() { }], + [$2], [$3]) +]) + +AC_DEFUN([CC_ATTRIBUTE_NONNULL], [ + CC_CHECK_ATTRIBUTE( + [nonnull], [nonnull()], + [void __attribute__((nonnull())) some_function(void *foo, void *bar) { foo = (void*)0; bar = (void*)0; }], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_UNUSED], [ + CC_CHECK_ATTRIBUTE( + [unused], , + [void some_function(void *foo, __attribute__((unused)) void *bar);], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_SENTINEL], [ + CC_CHECK_ATTRIBUTE( + [sentinel], , + [void some_function(void *foo, ...) __attribute__((sentinel));], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_DEPRECATED], [ + CC_CHECK_ATTRIBUTE( + [deprecated], , + [void some_function(void *foo, ...) __attribute__((deprecated));], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_ALIAS], [ + CC_CHECK_ATTRIBUTE( + [alias], [weak, alias], + [void other_function(void *foo) { } + void some_function(void *foo) __attribute__((weak, alias("other_function")));], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_MALLOC], [ + CC_CHECK_ATTRIBUTE( + [malloc], , + [void * __attribute__((malloc)) my_alloc(int n);], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_PACKED], [ + CC_CHECK_ATTRIBUTE( + [packed], , + [struct astructure { char a; int b; long c; void *d; } __attribute__((packed));], + [$1], [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_CONST], [ + CC_CHECK_ATTRIBUTE( + [const], , + [int __attribute__((const)) twopow(int n) { return 1 << n; } ], + [$1], [$2]) +]) + +AC_DEFUN([CC_FLAG_VISIBILITY], [ + AC_REQUIRE([CC_CHECK_WERROR]) + AC_CACHE_CHECK([if $CC supports -fvisibility=hidden], + [cc_cv_flag_visibility], + [cc_flag_visibility_save_CFLAGS="$CFLAGS" + CFLAGS="$CFLAGS $cc_cv_werror" + CC_CHECK_CFLAGS_SILENT([-fvisibility=hidden], + cc_cv_flag_visibility='yes', + cc_cv_flag_visibility='no') + CFLAGS="$cc_flag_visibility_save_CFLAGS"]) + + AS_IF([test "x$cc_cv_flag_visibility" = "xyes"], + [AC_DEFINE([SUPPORT_FLAG_VISIBILITY], 1, + [Define this if the compiler supports the -fvisibility flag]) + $1], + [$2]) +]) + +AC_DEFUN([CC_FUNC_EXPECT], [ + AC_REQUIRE([CC_CHECK_WERROR]) + AC_CACHE_CHECK([if compiler has __builtin_expect function], + [cc_cv_func_expect], + [ac_save_CFLAGS="$CFLAGS" + CFLAGS="$CFLAGS $cc_cv_werror" + AC_COMPILE_IFELSE([AC_LANG_SOURCE( + [int some_function() { + int a = 3; + return (int)__builtin_expect(a, 3); + }])], + [cc_cv_func_expect=yes], + [cc_cv_func_expect=no]) + CFLAGS="$ac_save_CFLAGS" + ]) + + AS_IF([test "x$cc_cv_func_expect" = "xyes"], + [AC_DEFINE([SUPPORT__BUILTIN_EXPECT], 1, + [Define this if the compiler supports __builtin_expect() function]) + $1], + [$2]) +]) + +AC_DEFUN([CC_ATTRIBUTE_ALIGNED], [ + AC_REQUIRE([CC_CHECK_WERROR]) + AC_CACHE_CHECK([highest __attribute__ ((aligned ())) supported], + [cc_cv_attribute_aligned], + [ac_save_CFLAGS="$CFLAGS" + CFLAGS="$CFLAGS $cc_cv_werror" + for cc_attribute_align_try in 64 32 16 8 4 2; do + AC_COMPILE_IFELSE([AC_LANG_SOURCE([ + int main() { + static char c __attribute__ ((aligned($cc_attribute_align_try))) = 0; + return c; + }])], [cc_cv_attribute_aligned=$cc_attribute_align_try; break]) + done + CFLAGS="$ac_save_CFLAGS" + ]) + + if test "x$cc_cv_attribute_aligned" != "x"; then + AC_DEFINE_UNQUOTED([ATTRIBUTE_ALIGNED_MAX], [$cc_cv_attribute_aligned], + [Define the highest alignment supported]) + fi +])