From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 9BB8328F33 for ; Tue, 6 Aug 2024 10:21:47 +0200 (CEST) Received: (qmail 27794 invoked by uid 550); 6 Aug 2024 08:21:43 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 27758 invoked from network); 6 Aug 2024 08:21:43 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=hacktivis.me; h=date :from:to:cc:message-id:references:mime-version:content-type :in-reply-to; s=20240308_115322; bh=cMOitUmLj7ln1i1Zv7u45Nun8Hii 1SzJSUkrSS4c+hY=; b=xd2jbXQ7gCfPfmxXOAsgxPkcJ6I5Wo6RDoY5uLOO8p5m MWRGCRhR/pFtcQRiOAw12XnCl82AGY3u6d3vAhyMtQJRqleR2jFpsZ0Eh5SizGZ6 ajC7zv3d6DGZDoJ7kqB0faPN5pHf92wR5XDeoi2GX0My4IeTKoT9dDULGhgLEHPS cT/pTTY4suwieBkOtG3b+eN06y8yDp9VbEa8TGJ85/dEyyKaiNAO1AkO2FReICMA WuVXD/4VdCloC3aN/AQLaJPpkr5IlPbYtFBza76nH8XUZAIRDsjcrdyWDFvQXst6 17lo1SeFXdAMRtCiQcl01IeEsCg708N0FdBCJRmUrXWLdv0YVSJWq7hHuiU+to6x mqd8BVSXoIytijZnnN2+0vmZ8avtoG7DizI6WFDg2KMP+tzw1Z+uFLSvCgTcFMBZ gvWhY+FBlVEMB40RGHhyfDQGmZ9gXeQHDJz6cPtGFM3+5qGkgK2Vk8ynm+4e75m3 wJtyxwIZRMIzX27D+5xcHtGW1uW/7T73lBt8JWab+xrdbmWCSY0tERIrbnOHNrio NtFhym9XNxa9f1S/qy7vpJZDqymjfGDZcgKLVd6xyU0ZwXmHlfi4lxolA+qeWEZU /V1cA28DnbZkRGb6GsLdA2W9Lo7hB1ipsidkd+rzot10nyyzZ3d2qwysk2KRSGs= DomainKey-Signature: a=rsa-sha1; c=nofws; d=hacktivis.me; h=date:from:to :cc:message-id:references:mime-version:content-type:in-reply-to; q=dns; s=20240308_115322; b=iKbIHOMLFJ+P3Q48ZTskq3ajZOLllHQdUlU On1Z+s6cPFSvheUwGdvbHxRe0LXqP9YqdqJDlbnyC9knBNy/R6LbDpIuuknIrdOS FNKONWyNQCdzxVIuga5Ekj0vJMOYrdQh3GdR6LuXQ4lpT2NOi3ZSoQv1/ikLbk2F QAj9aTQGJQLeRgg1bNNQfPd/6kvCIkR4AcN88jDmWv4ds5+7Uitnsk4JKTbSzV7h a2hJzaKu5fs+E8rWUfISTrGml3Nib+s4mc5d0Bqe6aMMvjZJhBL91cH5W1IMxEK7 VIHqOhrDtW89NRg6wuzNtEwHmSQymDAw/tPPRI32RMVYM5kRdF6lFwBiqNYISyEi 8/24kuFzmtQs/fW4ARj8sTvMrQxzfEwakXWx7yEIwktbL3H27/DOGpEZ9Crf+sUZ jfIsTCSgc6sZIgPpxIvNp2iuscH8vd/SkZwPXNk35Pq7HaP9IxMAzC49rd/9Y90T Adlt1//4lNBUthB3P9ccJeWiA4Nue3ovGJaPJVXPH0pLhhfq0M8WNhs56RQzzd31 QB6QOnQHksYDyNCKlsQMj1hc6LWB8urZHvSls35qzRJFahB/8LQ6AuLXPvWYGMjc hsLNNLDeneVFSvC22c0hPPvWTui07bdzpJn/KfsqP/XRswLqLfFuBcq/y2Ra05o1 I0O3gRNs= Date: Tue, 6 Aug 2024 10:21:33 +0200 From: "Haelwenn (lanodan) Monnier" To: Rich Felker Cc: musl@lists.openwall.com Message-ID: Mail-Followup-To: "Haelwenn (lanodan) Monnier" , Rich Felker , musl@lists.openwall.com References: <20240805065607.22897-1-contact@hacktivis.me> <20240805065607.22897-3-contact@hacktivis.me> <20240805182646.GC10433@brightrain.aerifal.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20240805182646.GC10433@brightrain.aerifal.cx> Subject: Re: [musl] [PATCH v2 3/3] signal: add str2sig(3) from POSIX.1-2024 [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" >> >> --- >> 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 >> +#include >> +#include >> +#include >> + >> +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.