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

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

[contact]

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)
+};
-- 
2.44.2

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

[contact]

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")
+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
+
+	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;
+	}
+
+	return -1;
+}
-- 
2.44.2

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

[Rich Felker]

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

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

[contact]

From: "Haelwenn (lanodan) Monnier" <contact@hacktivis.me>

---
 include/signal.h     |  1 +
 src/signal/str2sig.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 src/signal/str2sig.c

diff --git a/include/signal.h b/include/signal.h
index 217cfa08..fd486cd0 100644
--- a/include/signal.h
+++ b/include/signal.h
@@ -235,6 +235,7 @@ void psignal(int, const char *);
 
 #define SIG2STR_MAX sizeof("RTMIN+32")
 int sig2str(int signum, char *str);
+int str2sig(const char *restrict str, int *restrict pnum);
 
 #endif
 
diff --git a/src/signal/str2sig.c b/src/signal/str2sig.c
new file mode 100644
index 00000000..b6a4fc23
--- /dev/null
+++ b/src/signal/str2sig.c
@@ -0,0 +1,45 @@
+#include <signal.h>
+#include <string.h>
+#include <errno.h>
+#include <stdlib.h>
+
+int str2sig(const char *restrict str, int *restrict pnum)
+{
+	if (str[0] == '\0') return -1;
+
+	errno = 0;
+	long signum = strtol(str, NULL, 10);
+	if (errno == 0 && signum < _NSIG) return (*pnum = signum, 0);
+
+	if (strnlen(str, sizeof *__sys_signame) <= sizeof *__sys_signame)
+		for (int i = 0; i < sizeof __sys_signame/sizeof *__sys_signame; i++)
+			if (strncmp(str, __sys_signame[i], sizeof *__sys_signame) == 0)
+				return (*pnum = i, 0);
+
+	// signal aliases
+	if (strcmp(str, "IOT") == 0)
+		return (*pnum = SIGIOT, 0);
+	if (strcmp(str, "UNUSED") == 0)
+		return (*pnum = SIGUNUSED, 0);
+#if SIGPOLL != SIGIO
+	if (strcmp(str, "POLL") == 0)
+		return (*pnum = SIGPOLL, 0);
+#endif
+
+	if (strcmp(str, "RTMIN") == 0)
+		return (*pnum = SIGRTMIN, 0);
+	if (strcmp(str, "RTMAX") == 0)
+		return (*pnum = SIGRTMAX, 0);
+
+	if (strncmp(str, "RTMIN+", 6) == 0 || strncmp(str, "RTMAX-", 6) == 0)
+	{
+		errno = 0;
+		long sigrt = strtol(str+6, NULL, 10);
+		if(errno != 0 || sigrt < 1 || sigrt > SIGRTMAX - SIGRTMIN) return -1;
+
+		*pnum = str[5] == '+' ? SIGRTMIN + sigrt : SIGRTMAX - sigrt;
+		return 0;
+	}
+
+	return -1;
+}
-- 
2.44.2

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

[Rich Felker]

On Mon, Aug 05, 2024 at 08:56:07AM +0200, contact@hacktivis.me wrote:
> From: "Haelwenn (lanodan) Monnier" <contact@hacktivis.me>
> 
> ---
>  include/signal.h     |  1 +
>  src/signal/str2sig.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 src/signal/str2sig.c
> 
> diff --git a/include/signal.h b/include/signal.h
> index 217cfa08..fd486cd0 100644
> --- a/include/signal.h
> +++ b/include/signal.h
> @@ -235,6 +235,7 @@ void psignal(int, const char *);
>  
>  #define SIG2STR_MAX sizeof("RTMIN+32")
>  int sig2str(int signum, char *str);
> +int str2sig(const char *restrict str, int *restrict pnum);
>  
>  #endif
>  
> diff --git a/src/signal/str2sig.c b/src/signal/str2sig.c
> new file mode 100644
> index 00000000..b6a4fc23
> --- /dev/null
> +++ b/src/signal/str2sig.c
> @@ -0,0 +1,45 @@
> +#include <signal.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +
> +int str2sig(const char *restrict str, int *restrict pnum)
> +{
> +	if (str[0] == '\0') return -1;
> +
> +	errno = 0;
> +	long signum = strtol(str, NULL, 10);
> +	if (errno == 0 && signum < _NSIG) return (*pnum = signum, 0);
> +
> +	if (strnlen(str, sizeof *__sys_signame) <= sizeof *__sys_signame)
> +		for (int i = 0; i < sizeof __sys_signame/sizeof *__sys_signame; i++)
> +			if (strncmp(str, __sys_signame[i], sizeof *__sys_signame) == 0)
> +				return (*pnum = i, 0);
> +
> +	// signal aliases
> +	if (strcmp(str, "IOT") == 0)
> +		return (*pnum = SIGIOT, 0);
> +	if (strcmp(str, "UNUSED") == 0)
> +		return (*pnum = SIGUNUSED, 0);

If there were a lot of these I'd lean towards some table to avoid the
above, but there seem to be only two that fit this pattern, so what
you've done is probably best.

> +#if SIGPOLL != SIGIO
> +	if (strcmp(str, "POLL") == 0)
> +		return (*pnum = SIGPOLL, 0);
> +#endif

I think it's when they're equal, not when they're not-equal, that you
need to special-case this. Otherwise "POLL" won't be accepted on archs
where the values are equal. Of course you'd need to special-case both
if it weren't in the table, but see my comment on sig2str about
putting POLL in the table when not-equal.

> +	if (strcmp(str, "RTMIN") == 0)
> +		return (*pnum = SIGRTMIN, 0);
> +	if (strcmp(str, "RTMAX") == 0)
> +		return (*pnum = SIGRTMAX, 0);

These don't count in the "only two" I mentioned above because the
results are not a constant translation but runtime-variable.

This does raise a point that we could have SIGRTMIN and SIGRTMAX
expand to constants at musl build time, since the only way they could
change is by changing musl. My leaning is kinda not to do that,
though, because I've toyed with the idea of picking the
implementation-internal signals dynamically so that nesting in
qemu-user with some signals already stolen by the outer implementation
can work. I'm not sure if that's actually practical (it's more of a
pain than it sounds like, at least) but it's probably best to leave it
the way you've done it so as not to lock that possibility out.

> +	if (strncmp(str, "RTMIN+", 6) == 0 || strncmp(str, "RTMAX-", 6) == 0)
> +	{
> +		errno = 0;
> +		long sigrt = strtol(str+6, NULL, 10);
> +		if(errno != 0 || sigrt < 1 || sigrt > SIGRTMAX - SIGRTMIN) return -1;
> +
> +		*pnum = str[5] == '+' ? SIGRTMIN + sigrt : SIGRTMAX - sigrt;
> +		return 0;
> +	}
> +
> +	return -1;
> +}

The call to strtol isn't validating that there's not additional junk
after the digits, or that there's not a second sign character or
whitespace before the digits.

The canonical way to solve this is checking isdigit on the first char
and using the pend argument to see where parsing stopped, but in this
case, since the + or - is mandatory and already checked, you could
pass str+5 instead of str+6 to have it parse the + or - too. This
would require changing the arithmetic that follows, It also
complicates accepting 0 if you want to, but the current code rejects
0, so maybe that's okay?

It's also not clear if you want to accept over-long inputs like
RTMIN+000001. If not, checking that first char after + or - is in
range '1' to '9' would probably be the easiest.

Another possibility to consider is that interfacing with strtol is
sufficiently complicated that it's not worth bothering, and that
open-coding a-'0' or 10*(a-'0')+(b-'0') is easier (if you don't
actually intend to accept over-length representations).

Rich

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

[Haelwenn (lanodan) Monnier]

[2024-08-05 14:26:46-0400] Rich Felker:
>On Mon, Aug 05, 2024 at 08:56:07AM +0200, contact@hacktivis.me wrote:
>> From: "Haelwenn (lanodan) Monnier" <contact@hacktivis.me>
>>
>> ---
>>  include/signal.h     |  1 +
>>  src/signal/str2sig.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 46 insertions(+)
>>  create mode 100644 src/signal/str2sig.c
>>
>> diff --git a/include/signal.h b/include/signal.h
>> index 217cfa08..fd486cd0 100644
>> --- a/include/signal.h
>> +++ b/include/signal.h
>> @@ -235,6 +235,7 @@ void psignal(int, const char *);
>>
>>  #define SIG2STR_MAX sizeof("RTMIN+32")
>>  int sig2str(int signum, char *str);
>> +int str2sig(const char *restrict str, int *restrict pnum);
>>
>>  #endif
>>
>> diff --git a/src/signal/str2sig.c b/src/signal/str2sig.c
>> new file mode 100644
>> index 00000000..b6a4fc23
>> --- /dev/null
>> +++ b/src/signal/str2sig.c
>> @@ -0,0 +1,45 @@
>> +#include <signal.h>
>> +#include <string.h>
>> +#include <errno.h>
>> +#include <stdlib.h>
>> +
>> +int str2sig(const char *restrict str, int *restrict pnum)
>> +{
>> +	if (str[0] == '\0') return -1;
>> +
>> +	errno = 0;
>> +	long signum = strtol(str, NULL, 10);
>> +	if (errno == 0 && signum < _NSIG) return (*pnum = signum, 0);
>> +
>> +	if (strnlen(str, sizeof *__sys_signame) <= sizeof *__sys_signame)
>> +		for (int i = 0; i < sizeof __sys_signame/sizeof *__sys_signame; i++)
>> +			if (strncmp(str, __sys_signame[i], sizeof *__sys_signame) == 0)
>> +				return (*pnum = i, 0);
>> +
>> +	// signal aliases
>> +	if (strcmp(str, "IOT") == 0)
>> +		return (*pnum = SIGIOT, 0);
>> +	if (strcmp(str, "UNUSED") == 0)
>> +		return (*pnum = SIGUNUSED, 0);
>
>If there were a lot of these I'd lean towards some table to avoid the
>above, but there seem to be only two that fit this pattern, so what
>you've done is probably best.
>
>> +#if SIGPOLL != SIGIO
>> +	if (strcmp(str, "POLL") == 0)
>> +		return (*pnum = SIGPOLL, 0);
>> +#endif
>
>I think it's when they're equal, not when they're not-equal, that you
>need to special-case this. Otherwise "POLL" won't be accepted on archs
>where the values are equal. Of course you'd need to special-case both
>if it weren't in the table, but see my comment on sig2str about
>putting POLL in the table when not-equal.

Oups, right

>
>> +	if (strcmp(str, "RTMIN") == 0)
>> +		return (*pnum = SIGRTMIN, 0);
>> +	if (strcmp(str, "RTMAX") == 0)
>> +		return (*pnum = SIGRTMAX, 0);
>
>These don't count in the "only two" I mentioned above because the
>results are not a constant translation but runtime-variable.
>
>This does raise a point that we could have SIGRTMIN and SIGRTMAX
>expand to constants at musl build time, since the only way they could
>change is by changing musl. My leaning is kinda not to do that,
>though, because I've toyed with the idea of picking the
>implementation-internal signals dynamically so that nesting in
>qemu-user with some signals already stolen by the outer implementation
>can work. I'm not sure if that's actually practical (it's more of a
>pain than it sounds like, at least) but it's probably best to leave it
>the way you've done it so as not to lock that possibility out.
>
>> +	if (strncmp(str, "RTMIN+", 6) == 0 || strncmp(str, "RTMAX-", 6) == 0)
>> +	{
>> +		errno = 0;
>> +		long sigrt = strtol(str+6, NULL, 10);
>> +		if(errno != 0 || sigrt < 1 || sigrt > SIGRTMAX - SIGRTMIN) return -1;
>> +
>> +		*pnum = str[5] == '+' ? SIGRTMIN + sigrt : SIGRTMAX - sigrt;
>> +		return 0;
>> +	}
>> +
>> +	return -1;
>> +}
>
>The call to strtol isn't validating that there's not additional junk
>after the digits, or that there's not a second sign character or
>whitespace before the digits.
>
>The canonical way to solve this is checking isdigit on the first char
>and using the pend argument to see where parsing stopped, but in this
>case, since the + or - is mandatory and already checked, you could
>pass str+5 instead of str+6 to have it parse the + or - too. This
>would require changing the arithmetic that follows, It also
>complicates accepting 0 if you want to, but the current code rejects
>0, so maybe that's okay?
>
>It's also not clear if you want to accept over-long inputs like
>RTMIN+000001. If not, checking that first char after + or - is in
>range '1' to '9' would probably be the easiest.
>
>Another possibility to consider is that interfacing with strtol is
>sufficiently complicated that it's not worth bothering, and that
>open-coding a-'0' or 10*(a-'0')+(b-'0') is easier (if you don't
>actually intend to accept over-length representations).
>
>Rich

Given POSIX only specifies [1..SIGRTMAX-SIGRTMIN-1] I'm leaning towards
leaving RTMIN+0 and RTMAX-0 out unless it's basically gratis to support,
like when open-coding it.

And I think it's reasonable to expect only 1 or 2-digits for RTMIN+/RTMAX-,
although a for-loop on '0' could be done to skip zero-padding.

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

[Joakim Sindholt]

[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]

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.

[-- Attachment #2: sig2str.c --]
[-- Type: text/x-c++src, Size: 2279 bytes --]

#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;
}

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

[Rich Felker]

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