|
|
Message-ID: <20240805152903.GA10433@brightrain.aerifal.cx>
Date: Mon, 5 Aug 2024 11:29:03 -0400
From: Rich Felker <dalias@...c.org>
To: Joakim Sindholt <opensource@...sha.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH v2 1/3] src/signal/sys_signame.c: create hidden
value-name table of signals
On Mon, Aug 05, 2024 at 11:13:13AM +0200, Joakim Sindholt wrote:
> On Mon, 5 Aug 2024 08:56:05 +0200
> contact@...ktivis.me wrote:
>
> > From: "Haelwenn (lanodan) Monnier" <contact@...ktivis.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
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.