This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/6] Consolidate Linux sigprocmask implementation (BZ #22391)
- From: Rical Jasan <ricaljasan at pacific dot net>
- To: Zack Weinberg <zackw at panix dot com>
- Cc: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Sun, 5 Nov 2017 18:59:43 -0800
- Subject: Re: [PATCH 1/6] Consolidate Linux sigprocmask implementation (BZ #22391)
- Authentication-results: sourceware.org; auth=none
- References: <1509745249-11404-1-git-send-email-adhemerval.zanella@linaro.org> <CAKCAbMiS8shh4vSDRc_Gr2=RjL06VFOUX=yb1RyeHL+14cgCvA@mail.gmail.com>
On 11/05/2017 02:46 PM, Zack Weinberg wrote:
> On Fri, Nov 3, 2017 at 5:40 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> This patch consolidates the sigprocmask Linux syscall implementation on
>> sysdeps/unix/sysv/linux/sigprocmask.c
>
> This looks like a virtuous change overall, but I don't know the code
> well enough to singlehandedly review it. I'd like to provide some
> feedback on the manual change, though:
>
>> +On Linux pthreads internally uses some signals to implement asynchronous
>> +cancellation, effective ID synchronization for POSIX conformance, and
>> +posix timer management. These signals are filtered out on signal set
>> +function manipulation.
>
> This has grammar problems, and also, not all of the signal-set
> functions filter the special signals. For instance, sigfillset does,
> but sigaddset doesn't (if I'm reading the code right, anyway). And
> this isn't even the right place to document this, because the main
> reason why we need to document it is to explain why the kernel's idea
> of the real-time signal range differs from what glibc exposes to
> applications ... and the real-time signal range isn't documented at
> all, *headdesk*.
>
> So I suggest this _instead of_ the above change:
>
> diff --git a/manual/signal.texi b/manual/signal.texi
> index 9577ff091d..4f0ef59fb7 100644
> --- a/manual/signal.texi
> +++ b/manual/signal.texi
> @@ -235,6 +235,7 @@ defined. Since the signal numbers are allocated
> consecutively,
> * Job Control Signals:: Signals used to support job control.
> * Operation Error Signals:: Used to report operational system errors.
> * Miscellaneous Signals:: Miscellaneous Signals.
> +* Internally-Used Signals:: Signals used internally by the C library.
> * Signal Messages:: Printing a message describing a signal.
> @end menu
>
> @@ -794,6 +795,26 @@ in @ref{Signaling Another Process}.
> The default action is to terminate the process.
> @end deftypevr
>
> +@deftypevr Macro int SIGRTMIN
> +@deftypevrx Macro int SIGRTMAX
> +@standards{POSIX.1, signal.h}
> +@cindex real-time signals
> +The range of signal numbers @code{SIGRTMIN}, @code{SIGRTMIN+1},
> +@dots{}, @code{SIGRTMAX} is also set aside for you to use any way you
> +want. In addition, these signals (and no others) are guaranteed to
> +support @dfn{real-time} signal semantics, which unfortunately this
> +manual does not yet document.
I don't think we should say that at the end. It will be wrong someday
(hopefully), without having to do anything to it. It really just needs
a @pxref we don't have yet, so instead, say nothing, which will continue
to be correct both before and after real-time signal semantics are
documented, and after they are, the reference can be added to improve
the manual.
Maybe a bug report that real-time signal semantics aren't documented
would be a good place to both track the issue and keep a reminder to
update this paragraph.
> +
> +Unlike all of the other signal number macros, @code{SIGRTMIN} and
> +@code{SIGRTMAX} are not compile-time constants, because some operating
> +systems make the number of real-time signals tunable on a
> +per-installation or even per-process basis. However, POSIX guarantees
> +that there will be at least 8 signal numbers in this range.
> +
> +The default action for all signals in this range is to terminate the
> +process.
> +@end deftypevr
> +
> @deftypevr Macro int SIGWINCH
> @standards{BSD, signal.h}
> Window size change. This is generated on some systems (including GNU)
> @@ -817,6 +838,22 @@ to print some status information about the system
> and what the process
> is doing. Otherwise the default is to do nothing.
> @end deftypevr
>
> +@node Internally-Used Signals
> +@subsection Internally-Used Signals
> +@cindex internally used signals
> +
> +On some operating systems, @theglibc{} needs to use a few signals from
> +the ``true'' real-time range internally, to implement thread
> +cancellation, cross-thread effective ID synchronization, POSIX timer
> +management, etc. @Theglibc{} adjusts @code{SIGRTMIN} and
> +@code{SIGRTMAX} to exclude these signals, and it also takes steps to
> +prevent application code from altering their state: @code{sigprocmask}
> +cannot block them and @code{sigaction} cannot redefine their handlers,
> +for instance. Therefore, most programs do not need to know or care
> +about these signals. We mainly document their existence for the sake
> +of anyone who has ever wondered why there is a gap between the
> +highest-numbered ``normal'' signal and @code{SIGRTMIN} on Linux.
> +
> @node Signal Messages
> @subsection Signal Messages
> @cindex signal messages
Otherwise, I like the direction you took this, especially since it slips
in documentation for some previously undocumented macros along with the
concept.
Rical