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.3 required=5.0 tests=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 21FC026988 for ; Mon, 5 Aug 2024 20:26:59 +0200 (CEST) Received: (qmail 15985 invoked by uid 550); 5 Aug 2024 18:26:55 -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 15881 invoked from network); 5 Aug 2024 18:26:54 -0000 Date: Mon, 5 Aug 2024 14:26:46 -0400 From: Rich Felker To: contact@hacktivis.me Cc: musl@lists.openwall.com Message-ID: <20240805182646.GC10433@brightrain.aerifal.cx> References: <20240805065607.22897-1-contact@hacktivis.me> <20240805065607.22897-3-contact@hacktivis.me> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240805065607.22897-3-contact@hacktivis.me> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH v2 3/3] signal: add str2sig(3) from POSIX.1-2024 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. > + 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