Re: [PATCH] Fix a signal handler

From: Markus Elfring <markus.elfring@web.de>
Date: Feb 23, 2010, 08:55 UTC
Message-ID: <4B839811.6040109@web.de>
In-Reply-To: <7v1vgdgm02.fsf@alter.siamese.dyndns.org>

>     Subject: [PATCH] log --early-output: signal handler pedantic fix

I would prefer a correct and portable approach here instead of a "pedantic" one.
  ;-)

> I'd phrase the above like this:

It might be that the suggested commit message was too terse.

>     The behavior is undefined if the signal handler refers to any object
>     other than errno with static storage duration other than by assigning
>     a value to a static storage duration variable

I would not repeat the specification of undefined behaviour if a reference to a standard like POSIX will be sufficient.

> and that would be sufficiently clear without saying anything else.

It seems that we have got different opinions about the clarity of signal handling.

Show 5 quoted lines

> Your proposed log message also needs to make a good counter-argument why
> the above "we purposely avoid using sigatomic_t --- it is not worth the
> hassle of having to deal with systems that lack this type in practice" is
> worried too much, and it now is sensible to assume that everybody has
> sigatomic_t these days to allow us do "the right thing".

This data type is actually not used (because an underscore is missing in the name). ;-)

> It can be just as simple as 'Output from "git grep sigatomic_t" indicates
> that we are already using it.' but you need to say something, as this
> comment you are removing makes it clear that it was not a bug by mistake
> or ignorance, but instead was a deliberate choice.

Should I really add to the log message that there is another user for it like the source file "progress.c"?

> According to POSIX, "s-e-o" has to be "volatile sig_atomic_t".

How do you think about informations from a discussion on a topic like 'Is "volatile sig_atomic_t" redundant'? http://groups.google.de/group/comp.lang.c/browse_frm/thread/da3118a2d2c0737c/718dc093b83e03f8?#718dc093b83e03f8

> Also we do not explicitly initialize bss variables to zero or NULL.

If we would like to insist on the implementation of a strictly conforming program, the source code should be restructured even more. https://www.securecoding.cert.org/confluence/display/seccode/SIG31-C.+Do+not+access+or+modify+shared+objects+in+signal+handlers

The variable "show_early_output" should be moved to the source file "builtin-log.c" where it will become "static". Other means would be needed to transfer corresponding state changes to the function "path_name".

Regards, Markus