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.
cheers,
Pádraig.
