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 ++endifThe 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