On Wed, Jul 24, 2024 at 09:27:14PM GMT, Alejandro Colomar wrote: > POSIX leaves the value of *endptr unspecified if the base is invalid; > it is not unmodified. > > glibc and musl libc leave the object unmodified. However, the BSDs and > Bionic libc modify it to be nptr. > > That, combined with the fact that POSIX allows implementations of > strtol(3) to set EINVAL when no digits are found, makes it impossible to > portably check if the base was valid after the call. > > This bug was introduced after I incorrectly mentioned that strtol(3) > left *endptr unmodified; that's an implementation-defined behavior of > glibc and musl. > > Fixes: 64ddc975e72c (2024-07-18, "xstrtol: document and stray less from > strtol") > Fixes: 16b33e664942 (2024-07-19, "xstrtol: be more robust against odd > failures") > Link: <https://mail-index.netbsd.org/tech-misc/2024/07/20/msg000412.html>
I meant this link instead: <https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=58461> > Reported-by: Robert Elz <k...@munnari.oz.au> > Cc: Paul Eggert <egg...@cs.ucla.edu> > Cc: Bruno Haible <br...@clisp.org> > Signed-off-by: Alejandro Colomar <a...@kernel.org> > --- > > Hi Paul, Bruno, > > I haven't tested the patch. Please check thoroughly. I'm tired of > strtol(3). :) > > This was discussed in a NetBSD report about their strtoi(3) > implementation not being portable. I CCed Paul in a message there. > > Have a lovely day! > Alex > > lib/xstrtol.c | 34 ++++++++++++++++++---------------- > 1 file changed, 18 insertions(+), 16 deletions(-) > > diff --git a/lib/xstrtol.c b/lib/xstrtol.c > index 797d3e4dee..ccbfa82924 100644 > --- a/lib/xstrtol.c > +++ b/lib/xstrtol.c > @@ -71,9 +71,14 @@ strtol_error > __xstrtol (char const *nptr, char **endptr, int base, > __strtol_t *val, char const *valid_suffixes) > { > - char *t_ptr = nullptr; > + char *t_ptr; > char **p = endptr ? endptr : &t_ptr; > > + *p = (char *) nptr; > + > + if (base < 0 || base == 1 || 36 < base) > + return LONGINT_INVALID; > + > if (! TYPE_SIGNED (__strtol_t)) > { > char const *q = nptr; > @@ -81,20 +86,23 @@ __xstrtol (char const *nptr, char **endptr, int base, > while (isspace (ch)) > ch = *++q; > if (ch == '-') > - { > - *p = (char *) nptr; > - return LONGINT_INVALID; > - } > + return LONGINT_INVALID; > } > > errno = 0; > - __strtol_t tmp = __strtol (nptr, &t_ptr, base); > - if (!t_ptr) > - return LONGINT_INVALID; > - *p = t_ptr; > + __strtol_t tmp = __strtol (nptr, p, base); > + > strtol_error err = LONGINT_OK; > > - if (*p == nptr && (errno == 0 || errno == EINVAL)) > + if (errno == ERANGE) > + { > + err = LONGINT_OVERFLOW; > + } > + else if (errno != 0 && errno != EINVAL) > + { > + return LONGINT_INVALID; > + } > + else if (*p == nptr) > { > /* If there is no number but there is a valid suffix, assume the > number is 1. The string is invalid otherwise. */ > @@ -102,12 +110,6 @@ __xstrtol (char const *nptr, char **endptr, int base, > return LONGINT_INVALID; > tmp = 1; > } > - else if (errno != 0) > - { > - if (errno != ERANGE) > - return LONGINT_INVALID; > - err = LONGINT_OVERFLOW; > - } > > /* Let valid_suffixes == NULL mean "allow any suffix". */ > /* FIXME: update all callers except the ones that allow suffixes > -- > 2.45.2 > -- <https://www.alejandro-colomar.es/>
signature.asc
Description: PGP signature