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

Alejandro Colomar <[email protected]> Sat, 20 Jul 2024 21:03:30 +0200
Newsgroups dev.linux.lists.liba2i
Message-ID <[email protected]>
--xqoscqehcavmmfyf
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: Alejandro Colomar <[email protected]>, 
	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: [PATCH] strto[iu](3): Make the implementation portable
References: <v4tnmc6szseohaet4m5uwekzvc7gqbfbhlnuon36lutsms4o4h@57t3w2k6c5qc>
MIME-Version: 1.0
In-Reply-To: <v4tnmc6szseohaet4m5uwekzvc7gqbfbhlnuon36lutsms4o4h@57t3w2k6c5qc>

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.

Link: <https://lists.freedesktop.org/archives/libbsd/2024-July/000456.html>
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]>
---

Hi Christos,

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.

I didn't test it (I don't have a NetBSD build around), so please review
thoroughly.

BTW, the line=20

+	if (nptr =3D=3D e && (*rstatus =3D=3D 0 || *rstatus =3D=3D EINVAL))

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 errno
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.

Have a lovely day!
Alex

 common/lib/libc/stdlib/_strtoi.h | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/common/lib/libc/stdlib/_strtoi.h b/common/lib/libc/stdlib/_str=
toi.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 * __restric=
t 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 * __restric=
t 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 * __restric=
t 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 * __rest=
rict 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 * __restr=
ict 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;

base-commit: 7a4c6afd05862bf8c28f0730d8d9cd7e2dce2a50
--=20
2.45.2


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

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

iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmacCgIACgkQnowa+77/
2zIWNA/+N35BUXlzTp4a/e/ttbMBSxGuqZhG/+bqiq0NXof9raGbxW9sISXXT+7l
HvsvVDuZGtd1zmZEoZZgE5NfP/8aGL1eAO9xPXfzle2mmG7mEqHHI+lV93LPj0ks
cN1rTIJ6fDGqoWfJZ5qtstKQM7BydIQ6VrLclwqu7tDVX3BU/twYpyHz9xaej51Y
2lGLgQqsPbCUsNTLx7vMavqkaHS8Bm//9rzpqKPMdxAeEzirExFcNtrwKnpOncO2
Z1t5XvPqw4fvs1gLT/D6N5bNhKUuCbFuPb/P3Xalp+VAmzrCGaRKYgnHj/vbWWXX
C48jOQwE2Y75lbPB/bbY5HTz6e7ckiACXXKfOu/fhAP289iGwSZNjdyK1tTdz1ZE
ph5c4AcuWalGJerpxFpBHG5QKKk3IocDs5qJa6zQ9xeeAu0RiuGrQ9mnLc0LCgR4
d41ByE7Yod+v0QK3lDZow90XhDgDT+TzkBxA36Q+lXxWQf5SJPuo7efXcjP0FwTj
k6iKX+qreNJOaXHqf8b+vuO7Twegjn8792UyFjWByVPv4jzBdrlRnGoM+/+pUSKw
KcFNt422wS1KQpLKyW5dh88gXfRRhdQDnoOIqZSK+jcq1C5an66hJjeSvQ42yIJG
xvPR52zwBQFeCBtiCEfjCC3BpkghjyLbPB9O7JmmFj7b/OcurO8=
=6RjC
-----END PGP SIGNATURE-----

--xqoscqehcavmmfyf--