All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Elfring <Markus.Elfring@web.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH] Fix a signal handler
Date: Tue, 23 Feb 2010 09:55:45 +0100	[thread overview]
Message-ID: <4B839811.6040109@web.de> (raw)
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.


> 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

  reply	other threads:[~2010-02-23  8:55 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-02 16:14 Fix signal handler Markus Elfring
2010-02-02 20:58 ` Jeff King
2010-02-02 21:44   ` Markus Elfring
2010-02-02 22:32     ` Jeff King
2010-02-03 10:20       ` Markus Elfring
2010-02-03 10:29         ` Jeff King
2010-02-03 11:55           ` Markus Elfring
2010-02-03 13:12             ` Thomas Rast
2010-02-03 15:46               ` Markus Elfring
2010-02-03 15:52                 ` Shawn O. Pearce
2010-02-03 15:53                 ` Andreas Ericsson
2010-02-03 16:24                   ` Markus Elfring
2010-02-04  7:23                     ` Andreas Ericsson
2010-02-03 15:17             ` Jeff King
2010-02-03 16:04               ` Markus Elfring
2010-02-03 16:26                 ` Bill Lear
2010-02-09 18:01   ` Markus Elfring
2010-02-09 23:49     ` Daniel Barkalow
2010-02-10 17:08     ` [PATCH] " Markus Elfring
2010-02-10 17:14       ` Shawn O. Pearce
2010-02-10 17:35         ` Jeff King
2010-02-10 17:33       ` Jeff King
2010-02-13 13:30         ` Markus Elfring
2010-02-14  6:47           ` Jeff King
2010-02-14 10:19             ` Junio C Hamano
2010-02-18 16:31               ` Markus Elfring
2010-02-18 20:06                 ` Junio C Hamano
2010-02-19 11:05                   ` Markus Elfring
2010-02-22 12:10                   ` [PATCH] Fix a " Markus Elfring
2010-02-22 18:31                     ` Junio C Hamano
2010-02-23  8:55                       ` Markus Elfring [this message]
2010-02-23  9:10                         ` Markus Elfring
2010-02-23 21:48                         ` Junio C Hamano
2010-02-24 10:38                           ` Markus Elfring
2010-02-24 10:51                             ` Andreas Ericsson
2010-02-24 11:08                           ` Markus Elfring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B839811.6040109@web.de \
    --to=markus.elfring@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.