On 29/04/11 22:15, Pádraig Brady wrote:
> On 29/04/11 22:00, Jim Meyering wrote:
>> Eric Blake wrote:
>>> On a glibc system, Coverity gives this warning:
>>>
>>> Error: UNINIT:
>>> m4-1.4.16/lib/fatal-signal.c:183: var_decl: Declaring variable "action"
>>> without initializer.
>>> m4-1.4.16/lib/fatal-signal.c:198: uninit_use_in_call: Using uninitialized
>>> value "action": field "action".sa_restorer is uninitialized when calling
>>> "sigaction".
>>>
>>> For glibc, the warning is spurious (the sa_restorer field is
>>> silently overwritten inside the sigaction() implementation, so
>>> it doesn't matter what the user assigns there, and leaving it
>>> unitialized in the user is actually slightly more efficient).
>>> But it could very well be a valid bug report for other platforms,
>>> since POSIX allows other extension fields in struct sigaction;
>>> always setting all extension fields to 0 (via memset) will
>>> guarantee that those extension fields can't have arbitrary
>>> behavior due to being uninitialized.
>>>
>>> * lib/fatal-signal.c (install_handlers): Clear undocumented fields.
>>>
>>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>>> ---
>>>
>>> I'm not sure whether we should apply this patch. On the one
>>> hand, you can argue (as I did in the commit message) that
>>> uninitialized hidden members can be dangerous. On the other
>>> hand, you can argue that if you stick to just the POSIX-defined
>>> sa_flags values, then you can't trigger any extensions that
>>> would care about the contents of extension fields in the
>>> first palce.
>>>
>>> Thoughts on whether I should apply or ditch this patch?
>> ...
>>> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
>>> index aca9027..80ffda5 100644
>>> --- a/lib/fatal-signal.c
>>> +++ b/lib/fatal-signal.c
>>> @@ -182,6 +182,7 @@ install_handlers ()
>>> size_t i;
>>> struct sigaction action;
>>>
>>> + memset (&action, 0, sizeof action);
>>> action.sa_handler = &fatal_signal_handler;
>>> /* If we get a fatal signal while executing fatal_signal_handler, enter
>>> fatal_signal_handler recursively, since it is reentrant. Hence no
>>
>> I think the case for clearing the bits is a little
>> stronger than the one for leaving them uninitialized, and would
>> be even more in favor, it if only this initialization were portable:
>>
>> struct sigaction action = {0,};
>>
>> Now that gcc is fixed,
>> maybe we should use something like DECLARE_ZEROED_AUTO,
>> http://thread.gmane.org/gmane.comp.gnu.coreutils.general/1124/focus=1131
>> here in gnulib, too.
>
> Well {0,} is already used in gnulib.
>
> I'm going to change manywarnings.m4 so that
> -Wno-missing-field-initializers is added if needed.
>
> Then we don't need to worry about any of this.
Proposed patch attached.
cheers,
Pádraig.
>From 1d752a933227014f918eb180cf6c8abb77330d4b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 30 Apr 2011 00:16:01 +0100
Subject: [PATCH] manywarnings: add -Wno-missing-field-initializers if needed
* m4/manywarnings.m4 (gl_MANYWARN_ALL_GCC): Add the above
option if it's needed to allow initialization with { 0, },
which is the case with GCC before version 4.7
---
ChangeLog | 6 ++++++
m4/manywarnings.m4 | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 56 insertions(+), 1 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index cbd314e..e6d61bc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-04-30 Pádraig Brady <p...@draigbrady.com>
+
+ manywarnings: Add -Wno-missing-field-initializers if needed.
+ * m4/manywarnings.m4 (gl_MANYWARN_ALL_GCC): Add the above
+ option if it's needed to allow initialization with { 0, }
+
2011-04-29 Jim Meyering <meyer...@redhat.com>
vc-list-files: indent with spaces, not TABs
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index e928821..5b7439b 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -35,6 +35,49 @@ AC_DEFUN([gl_MANYWARN_COMPLEMENT],
# using gl_WARN_ADD if you want to make sure your gcc understands it.
AC_DEFUN([gl_MANYWARN_ALL_GCC],
[
+ dnl First, check if -Wno-missing-field-initializers is needed.
+ dnl -Wmissing-field-initializers is implied by -W, but that issues
+ dnl warnings with GCC version before 4.7, for the common idiom
+ dnl of initializing types on the stack to zero, using { 0, }
+ AC_REQUIRE([AC_PROG_CC])
+ if test -n "$GCC"; then
+
+ dnl First, check -W -Werror -Wno-missing-field-initializers is supported
+ dnl with the current $CC $CFLAGS $CPPFLAGS.
+ AC_MSG_CHECKING([whether -Wno-missing-field-initializers is supported])
+ AC_CACHE_VAL([gl_cv_nomfi_supported], [
+ gl_save_CFLAGS="$CFLAGS"
+ CFLAGS="$CFLAGS -W -Werror -Wno-missing-field-initializers"
+ AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM([[]], [[]])],
+ [gl_cv_nomfi_supported=yes],
+ [gl_cv_nomfi_supported=no])
+ CFLAGS="$gl_save_CFLAGS"])
+ AC_MSG_RESULT([$gl_cv_nomfi_supported])
+
+ if test "$gl_cv_nomfi_supported" = yes; then
+ dnl Now check whether -Wno-missing-field-initializers is needed
+ dnl for the { 0, } construct.
+ AC_MSG_CHECKING([whether -Wno-missing-field-initializers is needed])
+ AC_CACHE_VAL([gl_cv_nomfi_needed], [
+ gl_save_CFLAGS="$CFLAGS"
+ CFLAGS="$CFLAGS -W -Werror"
+ AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[void f (void)
+ {
+ typedef struct { int a; int b; } s_t;
+ s_t s1 = { 0, };
+ }
+ ]],
+ [[]])],
+ [gl_cv_nomfi_needed=no],
+ [gl_cv_nomfi_needed=yes])
+ CFLAGS="$gl_save_CFLAGS"])
+ AC_MSG_RESULT([$gl_cv_nomfi_needed])
+ fi
+ fi
+
gl_manywarn_set=
for gl_manywarn_item in \
-Wall \
@@ -104,5 +147,11 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
; do
gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item"
done
- $1=$gl_manywarn_set
+
+ # Disable the missing-field-initializers warning if needed
+ if test "$gl_cv_nomfi_needed" = yes; then
+ gl_manywarn_set="$gl_manywarn_set -Wno-missing-field-initializers"
+ fi
+
+ $1=$gl_manywarn_set
])
--
1.7.4
