Re: [PATCH] strto[iu](3): Make the implementation portable

--gpmc4wj3xmvofmdv
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: Guillem Jover <[email protected]>, christos <[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], [email protected]
Subject: Re: [PATCH] strto[iu](3): Make the implementation portable
References: <v4tnmc6szseohaet4m5uwekzvc7gqbfbhlnuon36lutsms4o4h@57t3w2k6c5qc>
 <[email protected]>
MIME-Version: 1.0
In-Reply-To: <[email protected]>

On Sat, Jul 20, 2024 at 09:03:34PM GMT, Alejandro Colomar wrote:
> POSIX allows systems that report EINVAL when no digits are found.  On
> such systems the only way to differentiate EINVAL and ECANCELED is to
> initialized the end pointer to NULL before the call.  On EINVAL cases,
> strto*max(3) will leave the pointer unmodified, so we'll read back the
> original NULL.  On ECANCELED cases, strto*max(3) will set it to nptr.
>=20
> Link: <https://lists.freedesktop.org/archives/libbsd/2024-July/000456.htm=
l>
> Cc: Guillem Jover <[email protected]>
> Cc: christos <[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]>
> Cc: <[email protected]>
> Signed-off-by: Alejandro Colomar <[email protected]>
> ---
>=20
> Hi Christos,
>=20
> I'm not sure if this patch is wanted in NetBSD.  It doesn't fix any bugs
> there.  It would be interesting for those pasting this code in other
> systems (which is currently done by libbsd).  Just in case you're
> interested, here it is.
>=20
> I didn't test it (I don't have a NetBSD build around), so please review
> thoroughly.
>=20
> BTW, the line=20
>=20
> +	if (nptr =3D=3D e && (*rstatus =3D=3D 0 || *rstatus =3D=3D EINVAL))
>=20
> You could also choose to simplify it as just `if (nptr =3D=3D e)`, since
> all existing implementations (AFAIK) only arrive at nptr =3D=3D e with er=
rno
> being either 0 or EINVAL.  However, for preventing ENOMEM or other
> strange errors that an implementation might add, those tests are there.
> Feel free to drop them (I didn't add them in my own strtoi(3)
> implementation).  I've kept them here for completeness (and because you
> already had a test `*rstatus =3D=3D 0` in that line, which was already
> superfluous, so I guessed you were preventing that kind of problems.

Self-correction: it was not superfluous.  It was there to prevent
reading `*endptr` when the base is invalid, which would have been UB,
since it was uninitialized.

>=20
> Have a lovely day!
> Alex
>=20
>  common/lib/libc/stdlib/_strtoi.h | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>=20
> diff --git a/common/lib/libc/stdlib/_strtoi.h b/common/lib/libc/stdlib/_s=
trtoi.h
> index b838608f6b52..bea6a9f285a7 100644
> --- a/common/lib/libc/stdlib/_strtoi.h
> +++ b/common/lib/libc/stdlib/_strtoi.h
> @@ -3,6 +3,7 @@
>  /*-
>   * Copyright (c) 1990, 1993
>   *	The Regents of the University of California.  All rights reserved.
> + * Copyright (c) 2024, Alejandro Colomar <[email protected]>
>   *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions
> @@ -69,7 +70,7 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __restr=
ict nptr,
>  	int serrno;
>  #endif
>  	__TYPE im;
> -	char *ep;
> +	char *e;
>  	int rep;
> =20
>  	_DIAGASSERT(hi >=3D lo);
> @@ -77,8 +78,7 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __restr=
ict nptr,
>  	_DIAGASSERT(nptr !=3D NULL);
>  	/* endptr may be NULL */
> =20
> -	if (endptr =3D=3D NULL)
> -		endptr =3D &ep;
> +	e =3D NULL;
> =20
>  	if (rstatus =3D=3D NULL)
>  		rstatus =3D &rep;
> @@ -90,9 +90,9 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __restr=
ict nptr,
> =20
>  #if defined(_KERNEL) || defined(_STANDALONE) || \
>      defined(HAVE_NBTOOL_CONFIG_H) || defined(BCS_ONLY)
> -	im =3D __WRAPPED(nptr, endptr, base);
> +	im =3D __WRAPPED(nptr, &e, base);
>  #else
> -	im =3D __WRAPPED_L(nptr, endptr, base, loc);
> +	im =3D __WRAPPED_L(nptr, &e, base, loc);
>  #endif
> =20
>  #if !defined(_KERNEL) && !defined(_STANDALONE)
> @@ -100,8 +100,11 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __re=
strict nptr,
>  	errno =3D serrno;
>  #endif
> =20
> +	if (endptr !=3D NULL && e !=3D NULL)
> +		*endptr =3D e;
> +
>  	/* No digits were found */
> -	if (*rstatus =3D=3D 0 && nptr =3D=3D *endptr)
> +	if (nptr =3D=3D e && (*rstatus =3D=3D 0 || *rstatus =3D=3D EINVAL))
>  		*rstatus =3D ECANCELED;
> =20
>  	if (im < lo) {
> @@ -117,7 +120,7 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __res=
trict nptr,
>  	}
> =20
>  	/* There are further characters after number */
> -	if (*rstatus =3D=3D 0 && **endptr !=3D '\0')
> +	if (*rstatus =3D=3D 0 && *e !=3D '\0')
>  		*rstatus =3D ENOTSUP;
> =20
>  	return im;
>=20
> base-commit: 7a4c6afd05862bf8c28f0730d8d9cd7e2dce2a50
> --=20
> 2.45.2
>=20



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

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

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

iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmac4loACgkQnowa+77/
2zLLSQ/9FEepXm2mDo1RBHApDVfJr+uok7niSz8qrCpXIsOBl+9yB+1qenrFYAdD
LvzisPg7M06/R0yveBIpymdZtlnu3u03hJB+B8NqJADCXdqTi21+r65u5fyhBhWU
+jEobFXgol/wdVhl81PZMAzQa5N/I6hAFxcEaog1NkKOVIUx9CWH2heCq/5RLKzp
Q7nTwP9heU4obmnA/7lBu7KKqP3A5M6gJa8bzevJxx4xJRSFozNHiKpHDmmjdxUD
6kUGOoRK7+T9EGIONI4SdfOcGHt9CiaZ3G5hI9OjDRJSkDdFlMhQAnXoZDHI25oY
5ng82MstiplWI2HyU/wehsxyJdzazyj+Wd3xe4RmovWHpcRIj5ju48K1Yg+cB1wy
BtfOaObXbMkOscqXcvXdRmZ59zYwVnQsMxEHsWqjXhgiQj40aF3NfvnM7BWrym5b
tJ7onpVoUedguax8m0wHQVPCzYgWaXNCj4kUcsObRshI+Om/dHfL0y90x9UAI+p4
V5xM1pQEZaZ1HmXbxwFYesdwyI6QWpg8NUUyDNYTJWehxYNkjAGX8iIR9UXnoJkz
wIZBVUq/RzomGJVQ/G8RK/KvMoGP9H0tyx8gLCrEJjVOi87nW0rYHvHZ1sUT+UG1
I57fknjQqYWEcKyDyBuBAA2E5e0UIjW4GQ38G7gTDseIUx+Cd2o=
=StSJ
-----END PGP SIGNATURE-----

--gpmc4wj3xmvofmdv--