Re: [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024

[Rich Felker <dalias@libc.org>]

On Sun, Aug 04, 2024 at 04:23:45PM +0200, Joakim Sindholt wrote:
> On Sun,  4 Aug 2024 14:41:44 +0200
> contact@hacktivis.me wrote:
> 
> > From: "Haelwenn (lanodan) Monnier" <contact@hacktivis.me>
> > 
> > ---
> >  include/signal.h     |  3 +++
> >  src/signal/sig2str.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 62 insertions(+)
> >  create mode 100644 src/signal/sig2str.c
> > 
> > diff --git a/include/signal.h b/include/signal.h
> > index c347f861..217cfa08 100644
> > --- a/include/signal.h
> > +++ b/include/signal.h
> > @@ -233,6 +233,9 @@ int pthread_kill(pthread_t, int);
> >  void psiginfo(const siginfo_t *, const char *);
> >  void psignal(int, const char *);
> >  
> > +#define SIG2STR_MAX sizeof("RTMIN+32")
> > +int sig2str(int signum, char *str);
> > +
> 
> Since SIG2STR_MAX is going to become ABI, do we need to oversize it? Are
> we allowed to oversize it?
> 
> >  #endif
> >  
> >  #if defined(_XOPEN_SOURCE) || defined(_BSD_SOURCE) || defined(_GNU_SOURCE)
> > diff --git a/src/signal/sig2str.c b/src/signal/sig2str.c
> > new file mode 100644
> > index 00000000..85f64ec6
> > --- /dev/null
> > +++ b/src/signal/sig2str.c
> > @@ -0,0 +1,59 @@
> > +#include <signal.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +
> > +int sig2str(int signum, char *str)
> > +{
> > +	const char *name = NULL;
> > +	switch(signum)
> > +	{
> > +		case SIGHUP: name = "HUP"; break;
> > +		case SIGINT: name = "INT"; break;
> > +		case SIGQUIT: name = "QUIT"; break;
> > +		case SIGILL: name = "ILL"; break;
> > +		case SIGTRAP: name = "TRAP"; break;
> > +		case SIGABRT: name = "ABRT"; break;
> > +		case SIGBUS: name = "BUS"; break;
> > +		case SIGFPE: name = "FPE"; break;
> > +		case SIGKILL: name = "KILL"; break;
> > +		case SIGUSR1: name = "USR1"; break;
> > +		case SIGSEGV: name = "SEGV"; break;
> > +		case SIGUSR2: name = "USR2"; break;
> > +		case SIGPIPE: name = "PIPE"; break;
> > +		case SIGALRM: name = "ALRM"; break;
> > +		case SIGTERM: name = "TERM"; break;
> > +		case SIGSTKFLT: name = "STKFLT"; break;
> > +		case SIGCHLD: name = "CHLD"; break;
> > +		case SIGCONT: name = "CONT"; break;
> > +		case SIGSTOP: name = "STOP"; break;
> > +		case SIGTSTP: name = "TSTP"; break;
> > +		case SIGTTIN: name = "TTIN"; break;
> > +		case SIGTTOU: name = "TTOU"; break;
> > +		case SIGURG: name = "URG"; break;
> > +		case SIGXCPU: name = "XCPU"; break;
> > +		case SIGXFSZ: name = "XFSZ"; break;
> > +		case SIGVTALRM: name = "VTALRM"; break;
> > +		case SIGPROF: name = "PROF"; break;
> > +		case SIGWINCH: name = "WINCH"; break;
> > +		case SIGIO: name = "IO"; break;
> > +		case SIGPWR: name = "PWR"; break;
> > +		case SIGSYS: name = "SYS"; break;
> > +	}
> 
> The spec says
> > If signum is a valid, supported signal number, is either less than
> > SIGRTMIN or greater than SIGRTMAX, and is not equal to one of the
> > symbolic constants listed in the table of signal numbers in
> > <signal.h>, the stored string shall uniquely identify the signal
> > number signum in an unspecified manner
> 
> We reserve 3 signals between SIGSYS and SIGRTMIN for timers, pthread
> cancellation, and the synccall machinery for setuid and the like.
> Are these "supported signal numbers" and if so, do we want to name them
> in the scheme of SIG32 or SIGTIMER?

They're not. Formally they're non-signals. All the sig src/signal/*.c
files treat them as invalid.

> > +	// macros to functions can't be in switch-case
> > +	if(signum == SIGRTMIN) name = "RTMIN";
> > +	if(signum == SIGRTMAX) name = "RTMAX";
> > +
> > +	if(SIGRTMIN+1 <= signum && signum <= SIGRTMAX-1)
> > +	{
> > +		if(snprintf(str, SIG2STR_MAX, "RTMIN+%i", signum-SIGRTMIN) < 0) return -1;
> 
> Using snprintf here to write out a 1 or 2 digit number isn't going to
> fly due to the sheer heft pulled in. Simply appending '0'+n/10
> conditionally and '0'+n%10 unconditionally to "RTMIN+" is probably the
> way to go here.

Indeed, I didn't think of the case where snprintf isn't already being
pulled in because that's uncommon, but it probably makes sense to do a
super simple open-coded version here..

> There's also a decision to be made as to whether we want to do RTMAX-n
> for the upper half of the signals, which the spec permits.

Doesn't seem particularly useful and kinda violates least-surprise.

Rich