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

--ghuoygcsxf4lpbxo
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]>
MIME-Version: 1.0
In-Reply-To: <[email protected]>

Hi Paul,

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")

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.

It was actually introduced in 3af47ca3f67 (1994-12-20, ".").

Please amend the Fixes tags to:

	Fixes: 3af47ca3f67 (1994-12-20, ".")
	Fixes: 64ddc975e72c (2024-07-18, "xstrtol: document and stray less from st=
rtol")

Sorry for the confusion!

Cheers,
Alex

> Fixes: 64ddc975e72c (2024-07-18, "xstrtol: document and stray less from s=
trtol")
> 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 base,
>        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 bas=
e,
>        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': case=
 'm':
>          case 'P': case 'Q': case 'R': case 'T': case 't': case 'Y': case=
 'Z':
> @@ -138,10 +143,10 @@ __xstrtol (char const *nptr, char **endptr, int bas=
e,
>               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 base,
>                }
>          }
> =20
> -      switch (**p)
> +      switch (*e)
>          {
>          case 'b':
>            overflow =3D bkm_scale (&tmp, 512);
> @@ -224,8 +229,10 @@ __xstrtol (char const *nptr, char **endptr, int base,
>          }
> =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
<https://www.alejandro-colomar.es/>

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

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

iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmaaZZwACgkQnowa+77/
2zJthhAAonsWS3gYgNHn43kCn1rFLydGkx2JxahPYqoZG62WsOom/X4WpMrNIP4E
RmH1h0mWO+tyyCtHYzDubTmUvjvgKzvJYt+i4vLgNcz5+AWsXSBzpczU4qAwxHf+
tpeT9X2SN/fcsF/rQYelfOZQvL1K2Z/sUUWHc3XpSqycrJmow3JnssYcE0AhRBC4
cfoeUJ3zgLyjpgSPPLG85KIjzAthcOrNTad0vrqDoVmIsJamkBl74cY1F6vrVXSH
pVtVlb0Z9m1CH6NjzDHgqr6o7oLmqtdc+SWgzqbr+pkw9S+mE7y+eQwhLAXMj3LR
07YMQakOTfh/uuFjG5OIB1lO6zSZzKSyG+QowUqriU9QukUObTgJITSB0y43cbLp
8+Y0pbMRdbuWSF+l/6hFe9ycGeo4e3h+NsHm90XJEwWA7uBvp+UQJ0xMkjkKPhTJ
CBs1ebxA8UH785/NCMm+VQ5uc/z/OpxpPXIX8GoH/sel6jk1BwkJJKE7Hfxl+ukv
UF2hVlEi0egfn158233/mbnhy2e81kqMqbUq19y4s8jbBGhY9gLg9Y+8T4ihE2je
qLwB21SmOL0ln9r/dddGEplFcJ8zB+k3Euzy0w2eTPGUxfn+n/8CNCzLCkHIfR4f
+zLRxTuuJCJ4IXu7fSSKdEOY2SgXej8THtkwbcR+/3SG34fyHys=
=53s+
-----END PGP SIGNATURE-----

--ghuoygcsxf4lpbxo--