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 C086F2A109 for ; Sun, 11 Aug 2024 04:54:38 +0200 (CEST) Received: (qmail 24186 invoked by uid 550); 11 Aug 2024 02:54:32 -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 24145 invoked from network); 11 Aug 2024 02:54:31 -0000 Date: Sat, 10 Aug 2024 22:54:24 -0400 From: Rich Felker To: contact@hacktivis.me Cc: musl@lists.openwall.com Message-ID: <20240811025424.GJ10433@brightrain.aerifal.cx> References: <20240809153423.30829-1-contact@hacktivis.me> <20240809153423.30829-3-contact@hacktivis.me> <20240811004314.GI10433@brightrain.aerifal.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240811004314.GI10433@brightrain.aerifal.cx> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH v3 3/3] signal: add str2sig(3) from POSIX.1-2024 On Sat, Aug 10, 2024 at 08:43:15PM -0400, Rich Felker wrote: > A few things I didn't catch before: > > On Fri, Aug 09, 2024 at 05:34:23PM +0200, contact@hacktivis.me wrote: > > From: "Haelwenn (lanodan) Monnier" > > > > --- > > include/signal.h | 1 + > > src/signal/str2sig.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 55 insertions(+) > > create mode 100644 src/signal/str2sig.c > > > > diff --git a/include/signal.h b/include/signal.h > > index 94ac29b1..5451424d 100644 > > --- a/include/signal.h > > +++ b/include/signal.h > > @@ -237,6 +237,7 @@ void psignal(int, const char *); > > // Bumped to 13 to be safe if a case like "SIGRTMIN+nnn" happens > > #define SIG2STR_MAX 13 > > int sig2str(int signum, char *str); > > +int str2sig(const char *__restrict str, int *__restrict pnum); > > As a matter of conformance, declarations in public headers can't have > non-reserved identifiers like str or pnum in them (these might already > have been #defined'd by the application). As a matter of policy, we > just don't use argument names or other "creative" content (including > comments) in the public headers at all, mainly because the headers > have been intended to be statements of fact not subject to copyright > (so a dynamic program using them is not a derived work and doesn't > rely on license at all). > > (There might be a few places where that's not entirely followed, but > if so they should be fixed.) > > > #endif > > > > diff --git a/src/signal/str2sig.c b/src/signal/str2sig.c > > new file mode 100644 > > index 00000000..e4c17c57 > > --- /dev/null > > +++ b/src/signal/str2sig.c > > @@ -0,0 +1,54 @@ > > +#include > > +#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); > > I don't think the errno check is either necessary or sufficient. As > written, this code won't reject something like "12x". Generally, you > need to pass a pend argument, like: > > char *end; > long signum = strtol(str, &end, 10); > if (!*end && signum < _NSIG) ... > > which checks that the whole input was consumed, and thereby also that > there was no error except possible range overflow (which would be > caught by range checks). > > However there's also a missing range check for negative values. This > could probably be handled by use of an unsigned type here, but unless > the intent is to accept inputs starting with + or -, the entire strtol > path should probably only be attempted if (isdigit(str[0])), in which > case negative can't happen. > > > + 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); > > I think the indexing for __sys_signame could be adjusted by 1 so > there's not an unused slot 0. The entries could also be 6-byte rather > than 7-byte. > > > + // 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) > > + { > > + if(!isdigit(str[6])) return -1; > > + > > + int sigrt = str[6]-'0'; > > + > > + if(str[7] != '\0') > > + { > > + if(!isdigit(str[7])) return -1; > > + > > + sigrt *= 10; > > + sigrt += str[7]-'0'; > > + } > > At this point you're missing a check that there's not further junk (or > more digits) after the second one. It could be something like: > > + if(!isdigit(str[7]) || str[8]) return -1; > > This isn't highly important, but the style str[8] vs str[8]!='\0' is > generally preferred in musl. OK, I've finally gone and dug up and read the specification: https://pubs.opengroup.org/onlinepubs/9799919799/functions/sig2str.html and it requires: "If str points to a string of the form "RTMIN+n", where n is a decimal representation of a number between 1 and SIGRTMAX-SIGRTMIN-1 inclusive, the stored value shall be equal to SIGRTMIN+n. "If str points to a string of the form "RTMAX-n", where n is a decimal representation of a number between 1 and SIGRTMAX-SIGRTMIN-1 inclusive, the stored value shall be equal to SIGRTMAX-n." So there is no requirement that the decimal number be in shortest form. As such this needs a loop or strtol (after checking isdigit). Rich