Re: [PATCH v1 1/2] xstrtol: Correctly handle an invalid base

--z5z3gi65dzon7x6j
Content-Type: text/plain; protected-headers=v1; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
From: Alejandro Colomar <[email protected]>
To: [email protected]
Cc: Paul Eggert <[email protected]>, Bruno Haible <[email protected]>, 
	=?utf-8?B?xJBvw6BuIFRy4bqnbiBDw7RuZw==?= Danh <[email protected]>, Eli Schwartz <[email protected]>, Sam James <[email protected]>, 
	Serge Hallyn <[email protected]>, Iker Pedrosa <[email protected]>, 
	Michael Vetter <[email protected]>, [email protected]
Subject: Re: [PATCH v1 1/2] xstrtol: Correctly handle an invalid base
References: <[email protected]>
 <[email protected]>
 <4tfcv4btjdbtp2dyekncwnw5flcgwq7rxftx3gleciehyxtbos@bix5rqzxrek2>
MIME-Version: 1.0
In-Reply-To: <4tfcv4btjdbtp2dyekncwnw5flcgwq7rxftx3gleciehyxtbos@bix5rqzxrek2>

On Fri, Jul 19, 2024 at 03:09:52PM GMT, Alejandro Colomar wrote:
> Hi Paul,
>=20
> On Fri, Jul 19, 2024 at 02:53:38PM GMT, Alejandro Colomar wrote:
> > strtol(3) doesn't set the end pointer if the base is invalid.  This
> > allows a caller to differentiate between "invalid base" (what
> > strtoi(3bsd) calls EINVAL) and an "no digits seen" (what strtoi(3bsd)
> > calls ECANCELED) in systems that report EINVAL on no digits seen (POSIX
> > allows this).
> >=20
> > 	strtol("foo", &e, 0);
> > 	strtol("0", &e, -1);
> >=20
> > The former call will set e =3D nptr.
> > The latter will leave e untouched.
> >=20
> > The caller has no other way to portably differentiate the calls.
> >=20
> > The way to differentiate those, thus, is to initialize e =3D NULL, to
> > allow reading it after the call.
> >=20
> > While doing this, change the behavior of this function to only set
> > *endptr if strtol(3) has set it, leaving it untouched otherwise.
> >=20
> > Fixes: 034a18049cbc (2014-12-20, "assure: new module")
>=20
> Self-correction: I had a typo while running git-blame(1), which led me
> to not find that the assure() call already existed --as assert()--
> before your commit, Paul.
>=20
> It was actually introduced in 3af47ca3f67 (1994-12-20, ".").
>=20
> Please amend the Fixes tags to:
>=20
> 	Fixes: 3af47ca3f67 (1994-12-20, ".")

Oops; For consistency, use 12 chars:

		3af47ca3f672

> 	Fixes: 64ddc975e72c (2024-07-18, "xstrtol: document and stray less from =
strtol")
>=20
> Sorry for the confusion!
>=20
> Cheers,
> Alex
>=20
> > Fixes: 64ddc975e72c (2024-07-18, "xstrtol: document and stray less from=
 strtol")
> > Cc: Paul Eggert <[email protected]>
> > Cc: Bruno Haible <[email protected]>
> > Cc: =C4=90o=C3=A0n Tr=E1=BA=A7n C=C3=B4ng Danh <[email protected]>
> > Cc: Eli Schwartz <[email protected]>
> > Cc: Sam James <[email protected]>
> > Cc: Serge Hallyn <[email protected]>
> > Cc: Iker Pedrosa <[email protected]>
> > Cc: Michael Vetter <[email protected]>
> > Cc: <[email protected]>
> > Signed-off-by: Alejandro Colomar <[email protected]>
> > ---
> >  lib/xstrtol.c | 35 +++++++++++++++++++++--------------
> >  1 file changed, 21 insertions(+), 14 deletions(-)
> >=20
> > diff --git a/lib/xstrtol.c b/lib/xstrtol.c
> > index c3145171f3..592673557f 100644
> > --- a/lib/xstrtol.c
> > +++ b/lib/xstrtol.c
> > @@ -71,9 +71,7 @@ strtol_error
> >  __xstrtol (char const *nptr, char **endptr, int base,
> >             __strtol_t *val, char const *valid_suffixes)
> >  {
> > -  char *t_ptr;
> > -  char **p =3D endptr ? endptr : &t_ptr;
> > -  *p =3D (char *) nptr;
> > +  char *e =3D NULL;
> > =20
> >    if (! TYPE_SIGNED (__strtol_t))
> >      {
> > @@ -82,14 +80,21 @@ __xstrtol (char const *nptr, char **endptr, int bas=
e,
> >        while (isspace (ch))
> >          ch =3D *++q;
> >        if (ch =3D=3D '-')
> > -        return LONGINT_INVALID;
> > +        {
> > +          if (endptr)
> > +            *endptr =3D (char *) nptr;
> > +          return LONGINT_INVALID;
> > +        }
> >      }
> > =20
> >    errno =3D 0;
> > -  __strtol_t tmp =3D __strtol (nptr, p, base);
> > +  __strtol_t tmp =3D __strtol (nptr, &e, base);
> >    strtol_error err =3D LONGINT_OK;
> > =20
> > -  if (*p =3D=3D nptr)
> > +  if (endptr && e)
> > +    *endptr =3D e;
> > +
> > +  if (e =3D=3D nptr)
> >      {
> >        /* If there is no number but there is a valid suffix, assume the
> >           number is 1.  The string is invalid otherwise.  */
> > @@ -113,19 +118,19 @@ __xstrtol (char const *nptr, char **endptr, int b=
ase,
> >        return err;
> >      }
> > =20
> > -  if (**p !=3D '\0')
> > +  if (*e !=3D '\0')
> >      {
> >        int xbase =3D 1024;
> >        int suffixes =3D 1;
> >        strtol_error overflow;
> > =20
> > -      if (!strchr (valid_suffixes, **p))
> > +      if (!strchr (valid_suffixes, *e))
> >          {
> >            *val =3D tmp;
> >            return err | LONGINT_INVALID_SUFFIX_CHAR;
> >          }
> > =20
> > -      switch (**p)
> > +      switch (*e)
> >          {
> >          case 'E': case 'G': case 'g': case 'k': case 'K': case 'M': ca=
se 'm':
> >          case 'P': case 'Q': case 'R': case 'T': case 't': case 'Y': ca=
se 'Z':
> > @@ -138,10 +143,10 @@ __xstrtol (char const *nptr, char **endptr, int b=
ase,
> >               power-of-1024.  */
> > =20
> >            if (strchr (valid_suffixes, '0'))
> > -            switch (p[0][1])
> > +            switch (e[1])
> >                {
> >                case 'i':
> > -                if (p[0][2] =3D=3D 'B')
> > +                if (e[2] =3D=3D 'B')
> >                    suffixes +=3D 2;
> >                  break;
> > =20
> > @@ -153,7 +158,7 @@ __xstrtol (char const *nptr, char **endptr, int bas=
e,
> >                }
> >          }
> > =20
> > -      switch (**p)
> > +      switch (*e)
> >          {
> >          case 'b':
> >            overflow =3D bkm_scale (&tmp, 512);
> > @@ -224,8 +229,10 @@ __xstrtol (char const *nptr, char **endptr, int ba=
se,
> >          }
> > =20
> >        err |=3D overflow;
> > -      *p +=3D suffixes;
> > -      if (**p)
> > +      e +=3D suffixes;
> > +      if (endptr)
> > +        *endptr =3D e;
> > +      if (*e)
> >          err |=3D LONGINT_INVALID_SUFFIX_CHAR;
> >      }
> > =20
> > --=20
> > 2.45.2
> >=20
>=20
>=20
>=20
> --=20
> <https://www.alejandro-colomar.es/>



--=20
<https://www.alejandro-colomar.es/>

--z5z3gi65dzon7x6j
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmaaZeEACgkQnowa+77/
2zKaAQ//ekCbcf1k+cpjOk0e04ToiLo8vW9rY7B01MP5LILEP0rZG4rsB7C3xpy3
j9AWtBQKUx3RQfh7d2GUbQlID/pij+jy658zFfwEK/D0QAoyY9xaDYUJhj/cdB/R
GwU9fqMZBGkWuxb2YK6ndGZOR6+jxYhPM2oIyC/inVSUNG/ZXxS+pr5GTot3Ap0Z
/8qe9D8CMyl2IjHk0aacjdefw+MyYsOifdGLsAcxGJ/QA7C0rQoYIMuKb2/InVKF
olO6Zc4J0Sh16Zpo1kCDLlie0+WvE6wToz7lwkukI7SWFa8iG3Ol6DlMMxQCOJvV
snWzV0KtMom2yaoOOaxN+uTAGNOvJ7en2L/PHcMdwIi79BZ8Q6LtKexkvgibNTp5
Er/j1UJl0fiel+GWQNNdF7Cfe9eaorLmMCYBDNZzzihy1YJN0bjttigit8HVXemN
XhF37KxRo14X3RaGmUNp7WoMsWIDK6a48gK5LPQoF1dtvov0VMN3/rPKM52ITGyX
oOAEbh4ljuXpoHqMs+1l32IoCA7vM7E4L1IN8ZId4hrw7oEzrDYey0ast9D0a5vj
jF27+ppYwR+72aLCzq9JDcmX/2YeUt11ihiq0fKeCrpYgl956j4qc7OHU2yqfX0G
F8+51vx0hs3LG+SI/30SxWxhtg5j2zc8QTYb0ghaCFghvIAzKKs=
=i8Pa
-----END PGP SIGNATURE-----

--z5z3gi65dzon7x6j--