Re: [PATCH] Fix a signal handler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>     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.


> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]