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

[Rich Felker <dalias@libc.org>]

On Mon, Aug 05, 2024 at 08:56:06AM +0200, contact@hacktivis.me wrote:
> From: "Haelwenn (lanodan) Monnier" <contact@hacktivis.me>
> 
> ---
>  include/signal.h     |  3 +++
>  src/signal/sig2str.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 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")

This definition is kinda misleading as written, since 32 isn't
actually the max; it can be much higher on mips where there are 127
signals. Originally I thought this made the bound wrong, but once you
take off the non-RT signals it still fits in 2 digits. It might be
better though just to write the literal size (so it's usable at
preprocessor level too; not sure if POSIX wants that but it's nice) or
even include some extra room just to be safe.

> +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..1967159c
> --- /dev/null
> +++ b/src/signal/sig2str.c
> @@ -0,0 +1,42 @@
> +#include <signal.h>
> +#include <string.h>
> +
> +int sig2str(int sig, char *str)
> +{
> +	if (sig <= 0) return -1;
> +
> +	if (sig <= SIGSYS)
> +		return (strcpy(str, __sys_signame[sig]), 0);
> +
> +	if (sig == SIGRTMIN)
> +		return (strcpy(str, "RTMIN"), 0);
> +	if (sig == SIGRTMAX)
> +		return (strcpy(str, "RTMAX"), 0);
> +
> +#if SIGPOLL != SIGIO
> +	if (sig == SIGPOLL)
> +		return (strcpy(str, "POLL"), 0);
> +#endif

Why isn't this one just in the table? It can be there conditional on
#if SIGPOLL != SIGIO, no?

> +	if (sig > SIGRTMIN && sig <= SIGRTMAX)
> +	{
> +		strcpy(str, "RTMIN+");
> +		int sigrt = sig-SIGRTMIN;
> +
> +		if (sigrt < 10)
> +		{
> +			str[6] = '0'+sigrt;
> +			str[7] = '\0';
> +		}
> +		else
> +		{
> +			str[6] = '0'+sigrt/10;
> +			str[7] = '0'+sigrt%10;
> +			str[8] = '\0';
> +		}
> +
> +		return 0;
> +	}

This might be prettier as something like:

	if (sigrt>=10) {
		*s++ = '0' + sigrt/10;
		sigrt %= 10;
	}
	*s++ = '0' + sigrt;

but it'd need a pointer (as in my example) or index var to write it
that way. I don't have a strong preference of this vs the way you
wrote it though, so whatever you like.

Rich