tech-misc archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



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 <guillem%hadrons.org@localhost>
Cc: christos <christos%netbsd.org@localhost>
Cc: Đoàn Trần Công Danh <congdanhqx%gmail.com@localhost>
Cc: Eli Schwartz <eschwartz93%gmail.com@localhost>
Cc: Sam James <sam%gentoo.org@localhost>
Cc: Serge Hallyn <serge%hallyn.com@localhost>
Cc: Iker Pedrosa <ipedrosa%redhat.com@localhost>
Cc: Michael Vetter <jubalh%iodoru.org@localhost>
Cc: <libbsd%lists.freedesktop.org@localhost>
Cc: <liba2i%lists.linux.dev@localhost>
Signed-off-by: Alejandro Colomar <alx%kernel.org@localhost>
---

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 

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

You could also choose to simplify it as just `if (nptr == e)`, since
all existing implementations (AFAIK) only arrive at nptr == 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 == 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/_strtoi.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 <alx%kernel.org@localhost>
  *
  * 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 * __restrict nptr,
 	int serrno;
 #endif
 	__TYPE im;
-	char *ep;
+	char *e;
 	int rep;
 
 	_DIAGASSERT(hi >= lo);
@@ -77,8 +78,7 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __restrict nptr,
 	_DIAGASSERT(nptr != NULL);
 	/* endptr may be NULL */
 
-	if (endptr == NULL)
-		endptr = &ep;
+	e = NULL;
 
 	if (rstatus == NULL)
 		rstatus = &rep;
@@ -90,9 +90,9 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __restrict nptr,
 
 #if defined(_KERNEL) || defined(_STANDALONE) || \
     defined(HAVE_NBTOOL_CONFIG_H) || defined(BCS_ONLY)
-	im = __WRAPPED(nptr, endptr, base);
+	im = __WRAPPED(nptr, &e, base);
 #else
-	im = __WRAPPED_L(nptr, endptr, base, loc);
+	im = __WRAPPED_L(nptr, &e, base, loc);
 #endif
 
 #if !defined(_KERNEL) && !defined(_STANDALONE)
@@ -100,8 +100,11 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __restrict nptr,
 	errno = serrno;
 #endif
 
+	if (endptr != NULL && e != NULL)
+		*endptr = e;
+
 	/* No digits were found */
-	if (*rstatus == 0 && nptr == *endptr)
+	if (nptr == e && (*rstatus == 0 || *rstatus == EINVAL))
 		*rstatus = ECANCELED;
 
 	if (im < lo) {
@@ -117,7 +120,7 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __restrict nptr,
 	}
 
 	/* There are further characters after number */
-	if (*rstatus == 0 && **endptr != '\0')
+	if (*rstatus == 0 && *e != '\0')
 		*rstatus = ENOTSUP;
 
 	return im;

base-commit: 7a4c6afd05862bf8c28f0730d8d9cd7e2dce2a50
-- 
2.45.2

Attachment: signature.asc
Description: PGP signature



Home | Main Index | Thread Index | Old Index