Add missing POSIX sig2str(3) & str2sig(3) calls #1696

ricardobranco777 · May 10, 2025

Add sig2str(3) & str2sig(3) calls defined in POSIX 1003.1-2024:

https://pubs.opengroup.org/onlinepubs/9799919799/functions/sig2str.html

Also add needed SIG2STR_MAX definition:

https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/signal.h.html

Alternative implementations:

Notes:

Except for manpage date, this patch applies cleanly on FreeBSD 14
Had to remove a smaller definition for SIG2STR_MAX here to use 32 as defined by Illumos

Next steps could be:

Modifying kill, fuser and other tools that need translation between signal names & numbers.
Add POSIX definition for NSIG_MAX in <limits.h>

Fixes https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=286714

github-actions · May 10, 2025

Thank you for taking the time to contribute to FreeBSD!
There is an issue that needs to be fixed:

Missing Signed-off-by lines^{dcbfc71, bec8831, 771be48, 4e3bc86, e9e983a, f57edc9}

ricardobranco777 · May 10, 2025

Tested with:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <signal.h>
#include <ctype.h>

static void
test(const char *arg)
{
	char buf[SIG2STR_MAX];
	int signum;
	char *end;

	if (isdigit((unsigned char)arg[0])) {
		signum = strtol(arg, &end, 10);
		if (sig2str(signum, buf) == 0)
			printf("sig2str(%d) = \"%s\"\n", signum, buf);
		else
			printf("sig2str(%d) = error\n", signum);
	} else {
		if (str2sig(arg, &signum) == 0)
			printf("str2sig(\"%s\") = %d\n", arg, signum);
		else
			printf("str2sig(\"%s\") = error\n", arg);
	}
}

int
main(int argc, char *argv[])
{
	for (int i = 1; i < argc; i++)
		test(argv[i]);

	return (0);
}

# Shall work
./str2sig HUP 1 USR2 31 RTMIN 65 RTMIN+1 66 RTMAX 126 RTMAX-1 125 RTMIN+30 95 RTMAX-30 96
str2sig("HUP") = 1
sig2str(1) = "HUP"
str2sig("USR2") = 31
sig2str(31) = "USR2"
str2sig("RTMIN") = 65
sig2str(65) = "RTMIN"
str2sig("RTMIN+1") = 66
sig2str(66) = "RTMIN+1"
str2sig("RTMAX") = 126
sig2str(126) = "RTMAX"
str2sig("RTMAX-1") = 125
sig2str(125) = "RTMAX-1"
str2sig("RTMIN+30") = 95
sig2str(95) = "RTMIN+30"
str2sig("RTMAX-30") = 96
sig2str(96) = "RTMAX-30"

# Shall fail
./str2sig XXX 0 -1 SIGXXX RTMIN-1 RTMAX+1
str2sig("XXX") = error
sig2str(0) = error
str2sig("-1") = error
str2sig("SIGXXX") = error
str2sig("RTMIN-1") = error
str2sig("RTMAX+1") = error

sys/sys/signal.h

lib/libc/gen/psignal.c

kostikbel

I think we should put the new functions into a new source file, in particular due to stdio use, as you noted.

sys/sys/signal.h

lib/libc/gen/psignal.c

lib/libc/gen/sig2str.c

kostikbel

@bsdimp please review and push

dag-erling · May 14, 2025

Could I ask you to please rewrite your test to use libatf-c, expand it to cover as many cases (including errors) as possible, and include it in the PR? (give it a name that ends in _test, put it in lib/libc/tests/gen/, don't forget to update the Makefile)

ricardobranco777 · May 14, 2025

Could I ask you to please rewrite your test to use libatf-c, expand it to cover as many cases (including errors) as possible, and include it in the PR? (give it a name that ends in _test, put it in lib/libc/tests/gen/, don't forget to update the Makefile)

Sure, I could do that. I have some experience with ATF on NetBSD.

dag-erling

Please also consider adding SSP instrumentation, see for instance d0b7445 which added instrumentation for realpath() which, like sig2str(), has an implied buffer size.

lib/libc/gen/psignal.3

lib/libc/gen/sig2str.c

include/signal.h

lib/libproc/libproc.h

lib/libc/gen/sig2str.c

usr.bin/fstat/fuser.c

dag-erling · May 14, 2025

Additionally, in bin/kill/kill.c, signame_to_signum() can be replaced with str2sig() and the -l logic can use sig2str() instead of accessing sys_signame[] directly.

BTW, both times strtol() is used in that file to parse a signal number, it assigns a long to an int without a range check, and the subsequent error check is incorrect. It would probably be better to use strtonum() instead. Also, it is bad form to use “illegal” in an error message; it should be replaced with something more neutral like “invalid” or “unrecognized”.

dag-erling · May 14, 2025

In bin/sh/trap.c, sigstring_to_signum() can be replaced with str2sig(). It still needs to check for EXIT, but since the function is only used once, that check can be moved to the caller. The other uses of sys_signame in bin/sh can be left as-is.

lib/libc/gen/sig2str.c

lib/libc/gen/psignal.3

lib/libproc/libproc.h

usr.bin/fstat/fuser.c

ricardobranco777 · May 19, 2025

$ cd /usr/tests/lib/libc/secure
$ kyua test fortify_signal_test
fortify_signal_test:sig2str_before_end  ->  passed  [0.002s]
fortify_signal_test:sig2str_end  ->  passed  [0.002s]
fortify_signal_test:sig2str_heap_after_end  ->  passed  [0.003s]
fortify_signal_test:sig2str_heap_before_end  ->  passed  [0.001s]
fortify_signal_test:sig2str_heap_end  ->  passed  [0.002s]
...

$ cd /usr/tests/lib/libc/gen
$ kyua test sig2str_test
sig2str_test:sig2str_invalid  ->  passed  [0.001s]
sig2str_test:sig2str_valid  ->  passed  [0.002s]
sig2str_test:str2sig_fullname  ->  passed  [0.002s]
sig2str_test:str2sig_invalid  ->  passed  [0.002s]
sig2str_test:str2sig_invalid_rt  ->  passed  [0.002s]
sig2str_test:str2sig_lowercase  ->  passed  [0.002s]
sig2str_test:str2sig_numeric  ->  passed  [0.002s]
sig2str_test:str2sig_rtmin_rtmax  ->  passed  [0.001s]
...

$ cd /usr/tests/bin/sh/builtins
$ kyua test functional_test:kill1
functional_test:kill1  ->  passed  [0.019s]
...
$ kyua test functional_test:kill2
functional_test:kill2  ->  passed  [0.017s]
...

Signed-off-by: Ricardo Branco <rbranco@suse.de>

sig2str(3)

bsdimp · Jun 5, 2025

I think this is ready to go and all the feedback has been addressed. Is that assessment correct?

bsdimp · Jun 11, 2025

Automated message from ghpr: Thank you for your submission. This PR has been merged to FreeBSD's branch. These changes will appear shortly on our GitHub mirror.

Reviewed by: imp, kib, des, jilles Pull Request: #1696

Signed-off-by: Ricardo Branco <rbranco@suse.de> Reviewed by: imp, kib, des, jilles Pull Request: #1696

Reviewed by: imp, kib, des, jilles Pull Request: #1696

sig2str(3) Reviewed by: imp, kib, des, jilles Pull Request: #1696 Closes: #1696

kostikbel reviewed May 10, 2025

View reviewed changes

sys/sys/signal.h Outdated Show resolved Hide resolved

lib/libc/gen/psignal.c Outdated Show resolved Hide resolved

lib/libc/gen/psignal.c Outdated Show resolved Hide resolved

lib/libc/gen/psignal.c Outdated Show resolved Hide resolved

ricardobranco777 force-pushed the sig2str branch 4 times, most recently from e1b5058 to 4b5d6cd Compare May 11, 2025 15:44

kostikbel reviewed May 11, 2025

View reviewed changes

sys/sys/signal.h Outdated Show resolved Hide resolved

lib/libc/gen/psignal.c Outdated Show resolved Hide resolved

lib/libc/gen/psignal.c Outdated Show resolved Hide resolved

lib/libc/gen/psignal.c Outdated Show resolved Hide resolved

ricardobranco777 force-pushed the sig2str branch 5 times, most recently from f19e29b to 1e77d5a Compare May 11, 2025 21:27

kostikbel reviewed May 11, 2025

View reviewed changes

lib/libc/gen/sig2str.c Outdated Show resolved Hide resolved

ricardobranco777 force-pushed the sig2str branch from 1e77d5a to a11ded8 Compare May 12, 2025 14:40

kostikbel approved these changes May 12, 2025

View reviewed changes

bsdimp self-assigned this May 13, 2025

bsdimp added needs-review Someone should look at this before proceeding ready Seems ready to go, subject to final sanity check labels May 13, 2025

dag-erling suggested changes May 14, 2025

View reviewed changes

ricardobranco777 force-pushed the sig2str branch 2 times, most recently from 6c878f2 to 733c7e4 Compare May 14, 2025 19:16

dag-erling reviewed May 14, 2025

View reviewed changes

lib/libc/gen/sig2str.c Outdated Show resolved Hide resolved

ricardobranco777 force-pushed the sig2str branch 2 times, most recently from a63aa06 to 5360d85 Compare May 14, 2025 20:52

dag-erling suggested changes May 15, 2025

View reviewed changes

lib/libc/gen/psignal.3 Outdated Show resolved Hide resolved

lib/libc/gen/psignal.3 Outdated Show resolved Hide resolved

lib/libc/gen/psignal.3 Outdated Show resolved Hide resolved

lib/libproc/libproc.h Show resolved Hide resolved

usr.bin/fstat/fuser.c Outdated Show resolved Hide resolved

ricardobranco777 force-pushed the sig2str branch 7 times, most recently from d31b9dc to a840ae5 Compare May 19, 2025 12:30

jillest approved these changes May 23, 2025

View reviewed changes

dag-erling self-requested a review May 24, 2025 10:00

dag-erling approved these changes May 24, 2025

View reviewed changes

ricardobranco777 added 7 commits June 1, 2025 10:35

Remove definition of SIG2STR_MAX from libproc.h

dcbfc71

Add the POSIX sig2str(3) & str2sig(3) calls

5308ff4

Signed-off-by: Ricardo Branco <rbranco@suse.de>

fstat: Use POSIX str2sig(3)

bec8831

Fix proc_signame() to use sig2str()

771be48

kill: Use POSIX str2sig()

4e3bc86

Add tests for sig2str() / str2sig()

e9e983a

include: ssp: fortify <signal.h>

f57edc9

sig2str(3)

ricardobranco777 force-pushed the sig2str branch from a840ae5 to f57edc9 Compare June 1, 2025 08:36

bsdimp added the merged Closed commit that's been merged label Jun 11, 2025

bsdimp closed this Jun 11, 2025

freebsd-git pushed a commit that referenced this pull request Jun 11, 2025

Remove definition of SIG2STR_MAX from libproc.h

6ff5a5b

Reviewed by: imp, kib, des, jilles Pull Request: #1696

freebsd-git pushed a commit that referenced this pull request Jun 11, 2025

Add the POSIX sig2str(3) & str2sig(3) calls

3d12567

Signed-off-by: Ricardo Branco <rbranco@suse.de> Reviewed by: imp, kib, des, jilles Pull Request: #1696

freebsd-git pushed a commit that referenced this pull request Jun 11, 2025

fstat: Use POSIX str2sig(3)

b826e88

Reviewed by: imp, kib, des, jilles Pull Request: #1696

freebsd-git pushed a commit that referenced this pull request Jun 11, 2025

Fix proc_signame() to use sig2str()

f942680

Reviewed by: imp, kib, des, jilles Pull Request: #1696

freebsd-git pushed a commit that referenced this pull request Jun 11, 2025

kill: Use POSIX str2sig()

36679f7

Reviewed by: imp, kib, des, jilles Pull Request: #1696

freebsd-git pushed a commit that referenced this pull request Jun 11, 2025

Add tests for sig2str() / str2sig()

d580567

Reviewed by: imp, kib, des, jilles Pull Request: #1696

freebsd-git pushed a commit that referenced this pull request Jun 11, 2025

include: ssp: fortify <signal.h>

b033445

sig2str(3) Reviewed by: imp, kib, des, jilles Pull Request: #1696 Closes: #1696

Search code, repositories, users, issues, pull requests...

Add missing POSIX sig2str(3) & str2sig(3) calls #1696

Add missing POSIX sig2str(3) & str2sig(3) calls #1696

Uh oh!

Conversation

ricardobranco777 commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ricardobranco777 commented May 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kostikbel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kostikbel left a comment

Choose a reason for hiding this comment

Uh oh!

dag-erling commented May 14, 2025

Uh oh!

ricardobranco777 commented May 14, 2025

Uh oh!

dag-erling left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dag-erling commented May 14, 2025

Uh oh!

dag-erling commented May 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ricardobranco777 commented May 19, 2025

Uh oh!

bsdimp commented Jun 5, 2025

Uh oh!

bsdimp commented Jun 11, 2025

Uh oh!

Uh oh!

ricardobranco777 commented May 10, 2025 •

edited

Loading

github-actions bot commented May 10, 2025 •

edited

Loading