Re: [musl] [PATCH v2 1/3] src/signal/sys_signame.c: create hidden value-name table of signals

[Rich Felker <dalias@libc.org>]

On Mon, Aug 05, 2024 at 11:13:13AM +0200, Joakim Sindholt wrote:
> On Mon,  5 Aug 2024 08:56:05 +0200
> contact@hacktivis.me wrote:
> 
> > From: "Haelwenn (lanodan) Monnier" <contact@hacktivis.me>
> > 
> > ---
> >  src/include/signal.h     |  2 ++
> >  src/signal/sys_signame.c | 41 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> >  create mode 100644 src/signal/sys_signame.c
> > 
> > diff --git a/src/include/signal.h b/src/include/signal.h
> > index bb566784..6bbc51d9 100644
> > --- a/src/include/signal.h
> > +++ b/src/include/signal.h
> > @@ -11,4 +11,6 @@ hidden void __restore_sigs(void *);
> >  
> >  hidden void __get_handler_set(sigset_t *);
> >  
> > +hidden extern const char __sys_signame[SIGSYS+1][sizeof("STKFLT")];
> > +
> >  #endif
> > diff --git a/src/signal/sys_signame.c b/src/signal/sys_signame.c
> > new file mode 100644
> > index 00000000..e086572c
> > --- /dev/null
> > +++ b/src/signal/sys_signame.c
> > @@ -0,0 +1,41 @@
> > +#include <signal.h>
> > +
> > +#define SIG(s) [SIG##s] = #s
> > +const char __sys_signame[SIGSYS+1][sizeof("STKFLT")] = {
> > +	SIG(HUP),
> > +	SIG(INT),
> > +	SIG(QUIT),
> > +	SIG(ILL),
> > +	SIG(TRAP),
> > +	SIG(ABRT),
> > +	SIG(BUS),
> > +	SIG(FPE),
> > +	SIG(KILL),
> > +	SIG(USR1),
> > +	SIG(SEGV),
> > +	SIG(USR2),
> > +	SIG(PIPE),
> > +	SIG(ALRM),
> > +	SIG(TERM),
> > +#if defined(SIGSTKFLT)
> > +	SIG(STKFLT),
> > +#endif
> > +#if defined(SIGEMT)
> > +	SIG(EMT),
> > +#endif
> > +	SIG(CHLD),
> > +	SIG(CONT),
> > +	SIG(STOP),
> > +	SIG(TSTP),
> > +	SIG(TTIN),
> > +	SIG(TTOU),
> > +	SIG(URG),
> > +	SIG(XCPU),
> > +	SIG(XFSZ),
> > +	SIG(VTALRM),
> > +	SIG(PROF),
> > +	SIG(WINCH),
> > +	SIG(IO),
> > +	SIG(PWR),
> > +	SIG(SYS)
> > +};
> 
> I know I'm being terribly annoying here but based on Rich's apparent
> preference for saving rodata space we could take out SIGSTKFLT, VTALRM,
> and WINCH. That leaves only 4-char signals with a handful of 3-char, and
> since we need to search through a few aliases and special cases anyway
> we could just add those 3 to a second table.
> 
> Also I hope there's a nicer way of making that table but this is what I
> came up with in a hurry.

> #include "signal.h"
> #include <errno.h>
> #include <string.h>
> #include <ctype.h>
> 
> #define SIG(s) [SIG##s-1] = #s
> 
> static const char map[][4] = {
> 	SIG(HUP),
> 	SIG(INT),
> 	SIG(QUIT),
> 	SIG(ILL),
> 	SIG(TRAP),
> 	SIG(ABRT),
> 	SIG(BUS),
> 	SIG(FPE),
> 	SIG(KILL),
> 	SIG(USR1),
> 	SIG(SEGV),
> 	SIG(USR2),
> 	SIG(PIPE),
> 	SIG(ALRM),
> 	SIG(TERM),
> 	SIG(CHLD),
> 	SIG(CONT),
> 	SIG(STOP),
> 	SIG(TSTP),
> 	SIG(TTIN),
> 	SIG(TTOU),
> 	SIG(URG),
> 	SIG(XCPU),
> 	SIG(XFSZ),
> 	SIG(PROF),
> 	SIG(IO),
> 	SIG(PWR),
> 	SIG(SYS),
> #ifdef SIGEMT
> 	SIG(EMT),
> #endif
> };
> 
> static const char other[] = {
> #ifdef SIGSTKFLT
> 	SIGSTKFLT,	'S','T','K','F','L','T', 0,
> #endif
> 	SIGVTALRM,	'V','T','A','L','R','M', 0,
> 	SIGWINCH,	'W','I','N','C','H', 0,
> 	RTMIN,		'R','T','M','I','N', 0,
> 	RTMAX,		'R','T','M','A','X', 0,
> 	/* aliases */
> 	SIGPOLL,	'P','O','L','L', 0,
> 	SIGIOT,		'I','O','T', 0,
> 	SIGUNUSED,	'U','N','U','S','E','D', 0,
> 	0
> };
> 
> int sig2str(int sig, char *str)
> {
> 	const char *p;
> 	int i, num;
> 
> 	if (sig > 0 && sig-1 < sizeof map/sizeof *map && *map[sig-1]) {
> 		for (i = 0; i < sizeof *map; i++)
> 			str[i] = map[sig-1][i];
> 		str[i] = 0;
> 		return 0;
> 	} else if (sig > RTMIN && sig < RTMAX) {
> 		for (i = 0; i < 6; i++)
> 			str[i] = "RTMIN+"[i];
> 		num = sig-RTMIN;
> 		if (num > 10)
> 			str[i++] = '0'+num/10;
> 		str[i++] = '0'+num%10;
> 		str[i] = 0;
> 		return 0;
> 	} else for (p = other, i = 0; i < 5; i++) {
> 		if (*p++ == sig) {
> 			for (i = 0; p[i]; i++)
> 				str[i] = p[i];
> 			return 0;
> 		}
> 		while (*p++)
> 			;
> 		p++;
> 	}
> 	errno = EINVAL;
> 	return -1;
> }
> 
> int str2sig(const char *restrict str, int *restrict sig)
> {
> 	const char *p;
> 	size_t len;
> 	int i, num;
> 
> 	len = strnlen(str, 5);
> 	if (len == 3 || len == 4)
> 		for (i = 0; i < sizeof map/sizeof *map; i++)
> 			if (strncmp(str, map[i], 4) == 0)
> 				return (*sig = i+1, 0);
> 	if (len <= 6)
> 		for (p = other; *p; p += len+1)
> 			if (strncmp(str, p+1, (len = strnlen(p+1, 6)+1)) == 0)
> 				return (*sig = *p, 0);
> 
> 	i = strncmp(str, "RTMIN+", 6)==0||strncmp(str, "RTMAX-", 6)==0?6:0;
> 	if (str[i] >= '1' && str[i] <= '9') {
> 		num = str[i++]-'0';
> 		if (isdigit(str[i]))
> 			num = num*10+str[i++]-'0';
> 		if (!str[i]) {
> 			if (isdigit(*str)) {
> 				if (num <= RTMAX)
> 					return (*sig = num, 0);
> 			} else if (num < RTMAX-RTMIN)
> 				return (*sig = str[5]=='+'?RTMIN+num:RTMAX-num, 0);
> 		}
> 	}
> 	/* errno = EINVAL ? */
> 	return -1;
> }

I don't see how this helps. You're adding at least 64 bytes of code
(.text) to save 64 bytes of table space...? My comment before was that
the waste from just using a 2D array [][6] is small enough that the
.text to do better would be larger than the .rodata, so it makes sense
to just do it the simplest way.

Rich