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

[Joakim Sindholt <opensource@zhasha.com>]

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?

> +	// 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.

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.

> +		return 0;
> +	}
> +
> +	if(name == NULL) return -1;
> +
> +	strcpy(str, name);
> +
> +	return 0;
> +}