mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024
@ 2024-08-04 12:41 contact
  2024-08-04 12:41 ` [musl] [PATCH 2/2] signal: add str2sig(3) " contact
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: contact @ 2024-08-04 12:41 UTC (permalink / raw)
  To: musl; +Cc: Haelwenn (lanodan) Monnier

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [musl] [PATCH 2/2] signal: add str2sig(3) from POSIX.1-2024
  2024-08-04 12:41 [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024 contact
@ 2024-08-04 12:41 ` contact
  2024-08-04 13:39   ` Pedro Falcato
  2024-08-04 14:23 ` [musl] [PATCH 1/2] signal: add sig2str(3) " Joakim Sindholt
  2024-08-04 14:29 ` Rich Felker
  2 siblings, 1 reply; 7+ messages in thread
From: contact @ 2024-08-04 12:41 UTC (permalink / raw)
  To: musl; +Cc: Haelwenn (lanodan) Monnier

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

---
 include/signal.h     |  1 +
 src/signal/str2sig.c | 72 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 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..6da2d4f4
--- /dev/null
+++ b/src/signal/str2sig.c
@@ -0,0 +1,72 @@
+#include <signal.h>
+#include <string.h>
+#include <errno.h>
+#include <stdlib.h>
+
+int str2sig(const char *restrict str, int *restrict pnum)
+{
+	errno = 0;
+	long signum = strtol(str, NULL, 10);
+	if(errno == 0 && signum < _NSIG)
+	{
+		*pnum = signum;
+		return 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;
+	}
+
+	int ret = -1;
+	if(strcmp(str, "HUP") == 0) ret = SIGHUP;
+	if(strcmp(str, "INT") == 0) ret = SIGINT;
+	if(strcmp(str, "QUIT") == 0) ret = SIGQUIT;
+	if(strcmp(str, "ILL") == 0) ret = SIGILL;
+	if(strcmp(str, "TRAP") == 0) ret = SIGTRAP;
+	if(strcmp(str, "ABRT") == 0) ret = SIGABRT;
+	if(strcmp(str, "IOT") == 0) ret = SIGIOT;
+	if(strcmp(str, "BUS") == 0) ret = SIGBUS;
+	if(strcmp(str, "FPE") == 0) ret = SIGFPE;
+	if(strcmp(str, "KILL") == 0) ret = SIGKILL;
+	if(strcmp(str, "USR1") == 0) ret = SIGUSR1;
+	if(strcmp(str, "SEGV") == 0) ret = SIGSEGV;
+	if(strcmp(str, "USR2") == 0) ret = SIGUSR2;
+	if(strcmp(str, "PIPE") == 0) ret = SIGPIPE;
+	if(strcmp(str, "ALRM") == 0) ret = SIGALRM;
+	if(strcmp(str, "TERM") == 0) ret = SIGTERM;
+	if(strcmp(str, "STKFLT") == 0) ret = SIGSTKFLT;
+	if(strcmp(str, "CHLD") == 0) ret = SIGCHLD;
+	if(strcmp(str, "CONT") == 0) ret = SIGCONT;
+	if(strcmp(str, "STOP") == 0) ret = SIGSTOP;
+	if(strcmp(str, "TSTP") == 0) ret = SIGTSTP;
+	if(strcmp(str, "TTIN") == 0) ret = SIGTTIN;
+	if(strcmp(str, "TTOU") == 0) ret = SIGTTOU;
+	if(strcmp(str, "URG") == 0) ret = SIGURG;
+	if(strcmp(str, "XCPU") == 0) ret = SIGXCPU;
+	if(strcmp(str, "XFSZ") == 0) ret = SIGXFSZ;
+	if(strcmp(str, "VTALRM") == 0) ret = SIGVTALRM;
+	if(strcmp(str, "PROF") == 0) ret = SIGPROF;
+	if(strcmp(str, "WINCH") == 0) ret = SIGWINCH;
+	if(strcmp(str, "IO") == 0) ret = SIGIO;
+	if(strcmp(str, "POLL") == 0) ret = SIGPOLL;
+	if(strcmp(str, "PWR") == 0) ret = SIGPWR;
+	if(strcmp(str, "SYS") == 0) ret = SIGSYS;
+	if(strcmp(str, "UNUSED") == 0) ret = SIGUNUSED;
+
+	if(strcmp(str, "RTMIN") == 0) ret = SIGRTMIN;
+	if(strcmp(str, "RTMAX") == 0) ret = SIGRTMAX;
+
+	if(ret == -1) return -1;
+
+	*pnum = ret;
+	return 0;
+}
-- 
2.44.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] [PATCH 2/2] signal: add str2sig(3) from POSIX.1-2024
  2024-08-04 12:41 ` [musl] [PATCH 2/2] signal: add str2sig(3) " contact
@ 2024-08-04 13:39   ` Pedro Falcato
  2024-08-04 14:02     ` Joakim Sindholt
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Falcato @ 2024-08-04 13:39 UTC (permalink / raw)
  To: musl; +Cc: Haelwenn (lanodan) Monnier

On Sun, Aug 4, 2024 at 1:42 PM <contact@hacktivis.me> wrote:
>
> From: "Haelwenn (lanodan) Monnier" <contact@hacktivis.me>
>
> ---
>  include/signal.h     |  1 +
>  src/signal/str2sig.c | 72 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 73 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..6da2d4f4
> --- /dev/null
> +++ b/src/signal/str2sig.c
> @@ -0,0 +1,72 @@
> +#include <signal.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +
> +int str2sig(const char *restrict str, int *restrict pnum)
> +{
> +       errno = 0;
> +       long signum = strtol(str, NULL, 10);
> +       if(errno == 0 && signum < _NSIG)
> +       {
> +               *pnum = signum;
> +               return 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;
> +       }
> +
> +       int ret = -1;
> +       if(strcmp(str, "HUP") == 0) ret = SIGHUP;
> +       if(strcmp(str, "INT") == 0) ret = SIGINT;
> +       if(strcmp(str, "QUIT") == 0) ret = SIGQUIT;
> +       if(strcmp(str, "ILL") == 0) ret = SIGILL;
> +       if(strcmp(str, "TRAP") == 0) ret = SIGTRAP;
> +       if(strcmp(str, "ABRT") == 0) ret = SIGABRT;
> +       if(strcmp(str, "IOT") == 0) ret = SIGIOT;
> +       if(strcmp(str, "BUS") == 0) ret = SIGBUS;
> +       if(strcmp(str, "FPE") == 0) ret = SIGFPE;
> +       if(strcmp(str, "KILL") == 0) ret = SIGKILL;
> +       if(strcmp(str, "USR1") == 0) ret = SIGUSR1;
> +       if(strcmp(str, "SEGV") == 0) ret = SIGSEGV;
> +       if(strcmp(str, "USR2") == 0) ret = SIGUSR2;
> +       if(strcmp(str, "PIPE") == 0) ret = SIGPIPE;
> +       if(strcmp(str, "ALRM") == 0) ret = SIGALRM;
> +       if(strcmp(str, "TERM") == 0) ret = SIGTERM;
> +       if(strcmp(str, "STKFLT") == 0) ret = SIGSTKFLT;
> +       if(strcmp(str, "CHLD") == 0) ret = SIGCHLD;
> +       if(strcmp(str, "CONT") == 0) ret = SIGCONT;
> +       if(strcmp(str, "STOP") == 0) ret = SIGSTOP;
> +       if(strcmp(str, "TSTP") == 0) ret = SIGTSTP;
> +       if(strcmp(str, "TTIN") == 0) ret = SIGTTIN;
> +       if(strcmp(str, "TTOU") == 0) ret = SIGTTOU;
> +       if(strcmp(str, "URG") == 0) ret = SIGURG;
> +       if(strcmp(str, "XCPU") == 0) ret = SIGXCPU;
> +       if(strcmp(str, "XFSZ") == 0) ret = SIGXFSZ;
> +       if(strcmp(str, "VTALRM") == 0) ret = SIGVTALRM;
> +       if(strcmp(str, "PROF") == 0) ret = SIGPROF;
> +       if(strcmp(str, "WINCH") == 0) ret = SIGWINCH;
> +       if(strcmp(str, "IO") == 0) ret = SIGIO;
> +       if(strcmp(str, "POLL") == 0) ret = SIGPOLL;
> +       if(strcmp(str, "PWR") == 0) ret = SIGPWR;
> +       if(strcmp(str, "SYS") == 0) ret = SIGSYS;
> +       if(strcmp(str, "UNUSED") == 0) ret = SIGUNUSED;
> +
> +       if(strcmp(str, "RTMIN") == 0) ret = SIGRTMIN;
> +       if(strcmp(str, "RTMAX") == 0) ret = SIGRTMAX;

To me it looks like the best way to implement these two functions is
to create a table of some sorts (indexed by signal, so array[SIGINT] =
"INT") and then use it.
str2sig would just iterate the whole array with strcmps, which is a
_little_ slow but this is probably not performance critical anyway.

Special casing would be required for RT signals, but that's no biggie.
What do you think?

--
Pedro

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] [PATCH 2/2] signal: add str2sig(3) from POSIX.1-2024
  2024-08-04 13:39   ` Pedro Falcato
@ 2024-08-04 14:02     ` Joakim Sindholt
  0 siblings, 0 replies; 7+ messages in thread
From: Joakim Sindholt @ 2024-08-04 14:02 UTC (permalink / raw)
  To: musl

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

On Sun, 4 Aug 2024 14:39:21 +0100 Pedro Falcato <pedro.falcato@gmail.com> wrote:
> To me it looks like the best way to implement these two functions is
> to create a table of some sorts (indexed by signal, so array[SIGINT] =
> "INT") and then use it.
> str2sig would just iterate the whole array with strcmps, which is a
> _little_ slow but this is probably not performance critical anyway.
> 
> Special casing would be required for RT signals, but that's no biggie.
> What do you think?

I was thinking the same thing for the sake of size.
There are a few more special cases though as some signal names refer to the
same signal numbers.

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

#include <signal.h>
#include <errno.h>
#include <string.h>
#include <ctype.h>

#define SIG(s) [SIG##s] = #s

static const char signames[][8] = {
    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(STKFLT),
    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(POLL),
    SIG(PWR),
    SIG(SYS),

    [32] = "TIMER",
    [33] = "CANCEL",
    [34] = "SYNCCALL",
    [35] = "RTMIN",
#define RTMIN 35
#define RTMAX SIGRTMAX
};

int sig2str(int sig, char *str)
{
    const char *name;
    int i, num;

    if (sig < sizeof signames/sizeof *signames) {
        for (i = 0; i < sizeof *signames; i++)
            str[i] = signames[sig][i];
        str[sizeof *signames] = 0;
        return 0;
    }
    if (sig == RTMAX) {
        for (i = 0; i < 6; i++)
            str[i] = "RTMAX"[i];
        return 0;
    }
    if (sig > RTMIN && sig <= RTMAX) {
        if (sig <= (RTMIN+RTMAX)/2) {
            name = "RTMIN+";
            num = sig-RTMIN;
        } else {
            name = sig==RTMAX?"RTMAX":"RTMAX-";
            num = RTMAX-sig;
        }
        for (i = 0; i < 6; i++)
            str[i] = name[i];
        if (num > 10)
            str[i++] = '0'+num/10;
        if (num > 0)
            str[i++] = '0'+num%10;
        str[i] = 0;
        return 0;
    }
    errno = EINVAL;
    return -1;
}

int str2sig(const char *restrict str, int *restrict sig)
{
    int i, num;

    if (strnlen(str, sizeof *signames) <= sizeof *signames)
        for (i = 0; i < sizeof signames/sizeof *signames; i++)
            if (strncmp(str, signames[i], sizeof *signames) == 0)
                return (*sig = i, 0);
    if (strcmp(str, "IO") == 0)
        return (*sig = SIGIO, 0);
    if (strcmp(str, "IOT") == 0)
        return (*sig = SIGIOT, 0);
    if (strcmp(str, "UNUSED") == 0)
        return (*sig = SIGUNUSED, 0);
    if (strcmp(str, "RTMAX") == 0)
        return (*sig = RTMAX, 0);

    if (strncmp(str, "RTMIN+", 6) == 0 || strncmp(str, "RTMAX-", 6) == 0)
        i = 6;
    else if (isdigit(*str))
        i = 0;
    else
        return -1;
    if (str[i] < '1' || str[i] > '9')
        return -1;
    num = str[i++]-'0';
    if (isdigit(str[i]))
        num = num*10+str[i++]-'0';
    if (str[i])
        return -1;
    if (*str != 'R')
        return num<=RTMAX?(*sig = num, 0):-1;
    else if (num < RTMAX-RTMIN)
        return (*sig = str[5]=='+'?RTMIN+num:RTMAX-num, 0);
    else
        return -1;
}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024
  2024-08-04 12:41 [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024 contact
  2024-08-04 12:41 ` [musl] [PATCH 2/2] signal: add str2sig(3) " contact
@ 2024-08-04 14:23 ` Joakim Sindholt
  2024-08-04 14:32   ` Rich Felker
  2024-08-04 14:29 ` Rich Felker
  2 siblings, 1 reply; 7+ messages in thread
From: Joakim Sindholt @ 2024-08-04 14:23 UTC (permalink / raw)
  To: musl

On Sun,  4 Aug 2024 14:41:44 +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);
> +

Since SIG2STR_MAX is going to become ABI, do we need to oversize it? Are
we allowed to oversize it?

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

The spec says
> If signum is a valid, supported signal number, is either less than
> SIGRTMIN or greater than SIGRTMAX, and is not equal to one of the
> symbolic constants listed in the table of signal numbers in
> <signal.h>, the stored string shall uniquely identify the signal
> number signum in an unspecified manner

We reserve 3 signals between SIGSYS and SIGRTMIN for timers, pthread
cancellation, and the synccall machinery for setuid and the like.
Are these "supported signal numbers" and if so, do we want to name them
in the scheme of SIG32 or SIGTIMER?

> +	// 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;

Using snprintf here to write out a 1 or 2 digit number isn't going to
fly due to the sheer heft pulled in. Simply appending '0'+n/10
conditionally and '0'+n%10 unconditionally to "RTMIN+" is probably the
way to go here.

There's also a decision to be made as to whether we want to do RTMAX-n
for the upper half of the signals, which the spec permits.

> +		return 0;
> +	}
> +
> +	if(name == NULL) return -1;
> +
> +	strcpy(str, name);
> +
> +	return 0;
> +}


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024
  2024-08-04 12:41 [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024 contact
  2024-08-04 12:41 ` [musl] [PATCH 2/2] signal: add str2sig(3) " contact
  2024-08-04 14:23 ` [musl] [PATCH 1/2] signal: add sig2str(3) " Joakim Sindholt
@ 2024-08-04 14:29 ` Rich Felker
  2 siblings, 0 replies; 7+ messages in thread
From: Rich Felker @ 2024-08-04 14:29 UTC (permalink / raw)
  To: contact; +Cc: musl

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024
  2024-08-04 14:23 ` [musl] [PATCH 1/2] signal: add sig2str(3) " Joakim Sindholt
@ 2024-08-04 14:32   ` Rich Felker
  0 siblings, 0 replies; 7+ messages in thread
From: Rich Felker @ 2024-08-04 14:32 UTC (permalink / raw)
  To: Joakim Sindholt; +Cc: musl

On Sun, Aug 04, 2024 at 04:23:45PM +0200, Joakim Sindholt wrote:
> On Sun,  4 Aug 2024 14:41:44 +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);
> > +
> 
> Since SIG2STR_MAX is going to become ABI, do we need to oversize it? Are
> we allowed to oversize it?
> 
> >  #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;
> > +	}
> 
> The spec says
> > If signum is a valid, supported signal number, is either less than
> > SIGRTMIN or greater than SIGRTMAX, and is not equal to one of the
> > symbolic constants listed in the table of signal numbers in
> > <signal.h>, the stored string shall uniquely identify the signal
> > number signum in an unspecified manner
> 
> We reserve 3 signals between SIGSYS and SIGRTMIN for timers, pthread
> cancellation, and the synccall machinery for setuid and the like.
> Are these "supported signal numbers" and if so, do we want to name them
> in the scheme of SIG32 or SIGTIMER?

They're not. Formally they're non-signals. All the sig src/signal/*.c
files treat them as invalid.

> > +	// 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;
> 
> Using snprintf here to write out a 1 or 2 digit number isn't going to
> fly due to the sheer heft pulled in. Simply appending '0'+n/10
> conditionally and '0'+n%10 unconditionally to "RTMIN+" is probably the
> way to go here.

Indeed, I didn't think of the case where snprintf isn't already being
pulled in because that's uncommon, but it probably makes sense to do a
super simple open-coded version here..

> There's also a decision to be made as to whether we want to do RTMAX-n
> for the upper half of the signals, which the spec permits.

Doesn't seem particularly useful and kinda violates least-surprise.

Rich

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-08-04 14:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-04 12:41 [musl] [PATCH 1/2] signal: add sig2str(3) from POSIX.1-2024 contact
2024-08-04 12:41 ` [musl] [PATCH 2/2] signal: add str2sig(3) " contact
2024-08-04 13:39   ` Pedro Falcato
2024-08-04 14:02     ` Joakim Sindholt
2024-08-04 14:23 ` [musl] [PATCH 1/2] signal: add sig2str(3) " Joakim Sindholt
2024-08-04 14:32   ` Rich Felker
2024-08-04 14:29 ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).