Thread (2 messages) 2 messages, 2 authors, 2025-06-26

Re: [PATCH v2 0/3] daemon: explicitly allow EINTR during poll()

From: Carlo Marcelo Arenas Belón <hidden>
Date: 2025-06-26 08:50:51

On Wed, Jun 25, 2025 at 09:07:11AM -0800, Junio C Hamano wrote:
"Carlo Marcelo Arenas Belón via GitGitGadget"
[off-list ref] writes:
quoted
     +@@ Makefile: include shared.mak
     + # when attempting to read from an fopen'ed directory (or even to fopen
     + # it at all).
     + #
     ++# Define USE_NON_POSIX_SIGNAL if don't have support for SA_RESTART or
     ++# prefer to use ANSI C signal() over POSIX sigaction()
     ++#
...
     ++ifdef USE_NON_POSIX_SIGNAL
     ++	COMPAT_CFLAGS += -DUSE_NON_POSIX_SIGNAL
     ++endif
The new symbol sounds like "POSIX does not have signal(2) but on
this platform we have a usable signal(2), so we use it here", but I
do not think that it is what we want to say (as POSIX inherits this
from ANSI C anyway).  More importantly, this "USE_X" sounds as if we
allow builders to set it and magically we stop using sigaction(2),
which is not what is going on.  We have tons of calls to both
signal(2) and sigaction(2), and we turn calls to signal(2) we have
in daemon.c to sigaction(2) but on some platforms their sigaction(2)
cannot do what we ask it to do, so we are stuck with signal(2) on
these platforms only for these calls in daemon.c.  It may be obvious
to those who develop and review this series, but not for anybody else.

Isn't the situation more like:

    We use sigaction(2) everywhere and have been happy with it in
    our code, but this topic discovered that on some platforms,
    their sigaction(2) does not do XYZ that everybody else's
    sigaction(2) does, so on them we need to fall back on the plain
    old signal(2) on selected code paths that we need XYZ out of the
    signal handling interface.

What is this XYZ that describes the characteristics of
signal/sigaction implementation on these platforms?  A name
constructed more like SIGACTION_LACKS_XYZ (hence we have to resort
to signal), possibly with a more appropriate verb than "lack", would
be less confusing.
sigaction(2) doesn't have any issues and it is indeed a better option
every time as it behaves the same in all platforms that have it.

the problems we have come from our codebase:

1) we have a hack to workaround the lack of support for SA_RESTART in
   some platforms, which sets it to 0 and allow us to compile as if it
   works.
2) Windows doesn't have sigaction() and their compability version needs
   updating if we would use it for this new code.

Keeping the fallback to use signal() isn't really needed, and is indeed
problematic, as it crashes in AIX and probably other SysIII derived
systems because of the hack Chris described for non BSD signal().

Carlo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help