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

[Rich Felker <dalias@libc.org>]

On Sun, Aug 04, 2024 at 02:41:44PM +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);
> +
>  #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;
> +	}
> +
> +	// 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;
> +
> +		return 0;
> +	}
> +
> +	if(name == NULL) return -1;
> +
> +	strcpy(str, name);
> +
> +	return 0;
> +}
> -- 
> 2.44.2

Could you do these with the approach of the existing strsignal.c? We
generally use that kind of single-concatenated-string table to avoid
ballooning numbers of dynamic relocations (and writable, nonshareable
memory) in each process shared libc.so is present in.

The sigmap[] machinery (only needed on one or two weird archs IIRC)
could be made external but hidden in a common file, so that it's not
duplicated again here.

Being that the standard signal names are bounded in length by 6, a
simple 2D array may be a better approach than having code to iterate
the concatenated multi-string like strsignal.c has. At most you waste
around 64 bytes this way, which is probably smaller than code, and
surely faster.

I see there's a little more logic needed for the realtime signals than
what strsignal.c has. I haven't yet read the spec but assuming the
above matches, I don't see a significantly lighter or simpler way than
what you'd doing. snprintf can't fail assuming valid format string and
non-overflowing-INT_MAX output, so error checking for it is not
needed.

Rich