On 2024-07-25 14:27, Bruno Haible wrote:
I have a hard time understanding the point
and purpose of what you are saying:
- In <https://lists.gnu.org/archive/html/bug-gnulib/2024-07/msg00171.html>
you removed the validation of 'base', saying:
"strtol can support base 1 as an extension, xstrtol now does
whatever the system does".
- In <https://lists.gnu.org/archive/html/bug-gnulib/2024-07/msg00238.html>
you agreed to leaving it unspecified whether xstrtol sets *endptr
in some case.
Fair enough, I suppose I added too much gingerbread there, so I
installed the attached patch to go back to the old way of doing things,
as that is simpler, less confusing, and a bit more efficient. The idea
is to not worry about hypothetical implementations or applications, just
real ones. The only thing missing from the old way of doing things, was
documentation that you shouldn't call xstrtol with a base other than 0
or 2 through 36, so the attached patch adds this to xstrtol.h.
documentation like "If you pass a base other than 0 or 2..36, you
can't expect a particular parsing, nor a particular error code, nor a
particular value for *endptr." is good.
I'm afraid we'll have to disagree on this particular case. It's OK for
behavior to be undefined when the base is bad, just as it's OK for it to
be undefined if the string is bad (e.g., when nptr == (char *) &errno).
There's no real-world hazard, and this email thread has taken more of
our time than the issue is worth.
I didn't adjust the recently-added Gnulib test cases even though they're
testing what is now documented to be undefined behavior, as they still
pass on the FreeBSD and Ubuntu platforms that I tried them on. If they
start failing we can remove them as needed. They're not testing anything
important, after all.From bd1e981434c98751b1106a1744e77a27317b52b3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 25 Jul 2024 17:24:20 -0700
Subject: [PATCH] xstrtol: remove the base-checking changes
* lib/xstrtol.c (__xstrtol): Stop worrying about hypothetical
implementations that are causing more confusion than the code is
worth. Instead, go back more to old way of doing things.
None of this matters for practical applications.
* lib/xstrtol.h: Document that behavior is undefined
if the base is negative, 1, or greater than 36.
* modules/xstrtol (Depends-on): Remove nullptr; no longer needed.
---
ChangeLog | 11 +++++++++++
lib/xstrtol.c | 9 +++------
lib/xstrtol.h | 2 ++
modules/xstrtol | 1 -
4 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 21b1840dec..0c35378606 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2024-07-25 Paul Eggert <egg...@cs.ucla.edu>
+
+ xstrtol: remove the base-checking changes
+ * lib/xstrtol.c (__xstrtol): Stop worrying about hypothetical
+ implementations that are causing more confusion than the code is
+ worth. Instead, go back more to old way of doing things.
+ None of this matters for practical applications.
+ * lib/xstrtol.h: Document that behavior is undefined
+ if the base is negative, 1, or greater than 36.
+ * modules/xstrtol (Depends-on): Remove nullptr; no longer needed.
+
2024-07-25 Bruno Haible <br...@clisp.org>
xstrtol, xstrtoll tests: Avoid test failure on FreeBSD.
diff --git a/lib/xstrtol.c b/lib/xstrtol.c
index 797d3e4dee..4e7c0fcfda 100644
--- a/lib/xstrtol.c
+++ b/lib/xstrtol.c
@@ -71,7 +71,7 @@ strtol_error
__xstrtol (char const *nptr, char **endptr, int base,
__strtol_t *val, char const *valid_suffixes)
{
- char *t_ptr = nullptr;
+ char *t_ptr;
char **p = endptr ? endptr : &t_ptr;
if (! TYPE_SIGNED (__strtol_t))
@@ -88,13 +88,10 @@ __xstrtol (char const *nptr, char **endptr, int base,
}
errno = 0;
- __strtol_t tmp = __strtol (nptr, &t_ptr, base);
- if (!t_ptr)
- return LONGINT_INVALID;
- *p = t_ptr;
+ __strtol_t tmp = __strtol (nptr, p, base);
strtol_error err = LONGINT_OK;
- if (*p == nptr && (errno == 0 || errno == EINVAL))
+ if (*p == nptr)
{
/* If there is no number but there is a valid suffix, assume the
number is 1. The string is invalid otherwise. */
diff --git a/lib/xstrtol.h b/lib/xstrtol.h
index 1fab748810..45c8921578 100644
--- a/lib/xstrtol.h
+++ b/lib/xstrtol.h
@@ -47,6 +47,8 @@ typedef enum strtol_error strtol_error;
- Return strtol_error, and store any result through an additional
TYPE *VAL pointer instead of returning the result.
- If TYPE is unsigned, reject leading '-'.
+ - Behavior is undefined if BASE is negative, 1, or greater than 36.
+ (In this respect xstrtol acts like the C standard, not like POSIX.)
- Accept an additional char const *VALID_SUFFIXES pointer to a
possibly-empty string containing allowed numeric suffixes,
which multiply the value. These include SI suffixes like 'k' and 'M';
diff --git a/modules/xstrtol b/modules/xstrtol
index 1f8a28fa6c..ef49dfc357 100644
--- a/modules/xstrtol
+++ b/modules/xstrtol
@@ -9,7 +9,6 @@ m4/xstrtol.m4
Depends-on:
intprops
-nullptr
stdckdint
stdint
--
2.43.0
