From: Alejandro Colomar <alx@kernel.org>
To: tech-misc@netbsd.org
Cc: "Alejandro Colomar" <alx@kernel.org>,
"Guillem Jover" <guillem@hadrons.org>,
christos <christos@netbsd.org>,
"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
"Eli Schwartz" <eschwartz93@gmail.com>,
"Sam James" <sam@gentoo.org>, "Serge Hallyn" <serge@hallyn.com>,
"Iker Pedrosa" <ipedrosa@redhat.com>,
"Michael Vetter" <jubalh@iodoru.org>,
libbsd@lists.freedesktop.org, liba2i@lists.linux.dev
Subject: [PATCH] strto[iu](3): Make the implementation portable
Date: Sat, 20 Jul 2024 21:03:30 +0200 [thread overview]
Message-ID: <20240720190321.550121-2-alx@kernel.org> (raw)
In-Reply-To: <v4tnmc6szseohaet4m5uwekzvc7gqbfbhlnuon36lutsms4o4h@57t3w2k6c5qc>
[-- Attachment #1: Type: text/plain, Size: 4087 bytes --]
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>
Cc: christos <christos@netbsd.org>
Cc: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Cc: Eli Schwartz <eschwartz93@gmail.com>
Cc: Sam James <sam@gentoo.org>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: <libbsd@lists.freedesktop.org>
Cc: <liba2i@lists.linux.dev>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
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>
*
* 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next parent reply other threads:[~2024-07-20 19:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <v4tnmc6szseohaet4m5uwekzvc7gqbfbhlnuon36lutsms4o4h@57t3w2k6c5qc>
2024-07-20 19:03 ` Alejandro Colomar [this message]
2024-07-21 10:26 ` [PATCH] strto[iu](3): Make the implementation portable Alejandro Colomar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240720190321.550121-2-alx@kernel.org \
--to=alx@kernel.org \
--cc=christos@netbsd.org \
--cc=congdanhqx@gmail.com \
--cc=eschwartz93@gmail.com \
--cc=guillem@hadrons.org \
--cc=ipedrosa@redhat.com \
--cc=jubalh@iodoru.org \
--cc=liba2i@lists.linux.dev \
--cc=libbsd@lists.freedesktop.org \
--cc=sam@gentoo.org \
--cc=serge@hallyn.com \
--cc=tech-misc@netbsd.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox