|
|
Message-ID: <20240804143228.GX10433@brightrain.aerifal.cx>
Date: Sun, 4 Aug 2024 10:32:28 -0400
From: Rich Felker <dalias@...c.org>
To: Joakim Sindholt <opensource@...sha.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024
On Sun, Aug 04, 2024 at 04:23:45PM +0200, Joakim Sindholt wrote:
> On Sun, 4 Aug 2024 14:41:44 +0200
> contact@...ktivis.me wrote:
>
> > From: "Haelwenn (lanodan) Monnier" <contact@...ktivis.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
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.