On 2024-07-18 12:53, Alejandro Colomar wrote:
I think it'd be common to assume that unless specifically
documented, you behave like POSIX's strtol(3), which produces defined
behavior for a base of 1.
Fair enough. I installed the attached Gnulib patch which addresses that
point, along with the other points you raised about xstrtol that I
understood. Since the system strtol can support base 1 as an extension,
xstrtol now does whatever the system does so there's no need for the
'assume'. (Not that any Gnulib-using programs care....) (Oh, and by the
way, I don't think I put that 'assume'/'assert'/whatever code in there
decades ago as I'm not a fan of that sort of thing....)
PS. Sorry about missing a space in the ChangeLog entry - I fixed that in
a followup patch.From 64ddc975e72cb55d2b2d755c25603bd70312aa5e Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 18 Jul 2024 17:28:36 -0700
Subject: [PATCH] xstrtol: document and stray less from strtol
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This patch alters xstrtol behavior slightly, in areas are not
likely to affect any real callers, by making xstrtol behave more
like the system strtol. In particular, it lets xstrtol support
bases other than those required by POSIX, if the underlying
implementation does; this removes the need for an ‘assert’.
It also uses ‘restrict’ like the underlying strtol does.
The patch also documents xstrtol in xstrtol.h.
Problems reported by AlejandroColomar in:
https://lists.gnu.org/r/bug-gnulib/2024-07/msg00159.html
* lib/xstrtol.c: Do not include stdio.h or assure.h.
(__xstrtol): Use same parameter names as POSIX, to make it
easier for outsiders to follow. Assume C99 decls after statement.
Do not require the base to be 0, or 2-36, as the underlying
implementation is allowed to support other bases.
Do not assume isspace preserves errno.
* lib/xstrtol.h (_DECLARE_XSTRTOL): Declare pointers to be
‘restrict’, for compatibility with C.
* m4/xstrtol.m4 (gl_XSTRTOL): Require AC_C_RESTRICT.
* modules/xstrtol (Depends-on): Remove ‘assure’.
---
ChangeLog | 23 +++++++++++++++++++
lib/xstrtol.c | 59 +++++++++++++++++++------------------------------
lib/xstrtol.h | 17 +++++++++++++-
m4/xstrtol.m4 | 3 ++-
modules/xstrtol | 1 -
5 files changed, 64 insertions(+), 39 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 833ecd6d80..1728846392 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,26 @@
+2024-07-18 Paul Eggert <egg...@cs.ucla.edu>
+
+ xstrtol: document and stray less from strtol
+ This patch alters xstrtol behavior slightly, in areas are not
+ likely to affect any real callers, by making xstrtol behave more
+ like the system strtol. In particular, it lets xstrtol support
+ bases other than those required by POSIX, if the underlying
+ implementation does; this removes the need for an ‘assert’.
+ It also uses ‘restrict’ like the underlying strtol does.
+ The patch also documents xstrtol in xstrtol.h.
+ Problems reported by AlejandroColomar in:
+ https://lists.gnu.org/r/bug-gnulib/2024-07/msg00159.html
+ * lib/xstrtol.c: Do not include stdio.h or assure.h.
+ (__xstrtol): Use same parameter names as POSIX, to make it
+ easier for outsiders to follow. Assume C99 decls after statement.
+ Do not require the base to be 0, or 2-36, as the underlying
+ implementation is allowed to support other bases.
+ Do not assume isspace preserves errno.
+ * lib/xstrtol.h (_DECLARE_XSTRTOL): Declare pointers to be
+ ‘restrict’, for compatibility with C.
+ * m4/xstrtol.m4 (gl_XSTRTOL): Require AC_C_RESTRICT.
+ * modules/xstrtol (Depends-on): Remove ‘assure’.
+
2024-07-18 Bruno Haible <br...@clisp.org>
abort-debug: Don't assume that signal SIGABRT is unmasked and unhandled.
diff --git a/lib/xstrtol.c b/lib/xstrtol.c
index 575c16d45f..c3145171f3 100644
--- a/lib/xstrtol.c
+++ b/lib/xstrtol.c
@@ -30,10 +30,6 @@
#include "xstrtol.h"
-/* Some pre-ANSI implementations (e.g. SunOS 4)
- need stderr defined if assertion checking is enabled. */
-#include <stdio.h>
-
#include <ctype.h>
#include <errno.h>
#include <limits.h>
@@ -45,7 +41,6 @@
# include <inttypes.h>
#endif
-#include "assure.h"
#include "intprops.h"
static strtol_error
@@ -72,26 +67,17 @@ bkm_scale_by_power (__strtol_t *x, int base, int power)
return err;
}
-/* FIXME: comment. */
-
strtol_error
-__xstrtol (const char *s, char **ptr, int strtol_base,
- __strtol_t *val, const char *valid_suffixes)
+__xstrtol (char const *nptr, char **endptr, int base,
+ __strtol_t *val, char const *valid_suffixes)
{
char *t_ptr;
- char **p;
- __strtol_t tmp;
- strtol_error err = LONGINT_OK;
-
- assure (0 == strtol_base || (2 <= strtol_base && strtol_base <= 36));
-
- p = (ptr ? ptr : &t_ptr);
-
- errno = 0;
+ char **p = endptr ? endptr : &t_ptr;
+ *p = (char *) nptr;
if (! TYPE_SIGNED (__strtol_t))
{
- const char *q = s;
+ char const *q = nptr;
unsigned char ch = *q;
while (isspace (ch))
ch = *++q;
@@ -99,16 +85,17 @@ __xstrtol (const char *s, char **ptr, int strtol_base,
return LONGINT_INVALID;
}
- tmp = __strtol (s, p, strtol_base);
+ errno = 0;
+ __strtol_t tmp = __strtol (nptr, p, base);
+ strtol_error err = LONGINT_OK;
- if (*p == s)
+ if (*p == nptr)
{
/* If there is no number but there is a valid suffix, assume the
number is 1. The string is invalid otherwise. */
- if (valid_suffixes && **p && strchr (valid_suffixes, **p))
- tmp = 1;
- else
+ if (! (valid_suffixes && *nptr && strchr (valid_suffixes, *nptr)))
return LONGINT_INVALID;
+ tmp = 1;
}
else if (errno != 0)
{
@@ -128,7 +115,7 @@ __xstrtol (const char *s, char **ptr, int strtol_base,
if (**p != '\0')
{
- int base = 1024;
+ int xbase = 1024;
int suffixes = 1;
strtol_error overflow;
@@ -160,7 +147,7 @@ __xstrtol (const char *s, char **ptr, int strtol_base,
case 'B':
case 'D': /* 'D' is obsolescent */
- base = 1000;
+ xbase = 1000;
suffixes++;
break;
}
@@ -184,39 +171,39 @@ __xstrtol (const char *s, char **ptr, int strtol_base,
break;
case 'E': /* exa or exbi */
- overflow = bkm_scale_by_power (&tmp, base, 6);
+ overflow = bkm_scale_by_power (&tmp, xbase, 6);
break;
case 'G': /* giga or gibi */
case 'g': /* 'g' is undocumented; for compatibility only */
- overflow = bkm_scale_by_power (&tmp, base, 3);
+ overflow = bkm_scale_by_power (&tmp, xbase, 3);
break;
case 'k': /* kilo */
case 'K': /* kibi */
- overflow = bkm_scale_by_power (&tmp, base, 1);
+ overflow = bkm_scale_by_power (&tmp, xbase, 1);
break;
case 'M': /* mega or mebi */
case 'm': /* 'm' is undocumented; for compatibility only */
- overflow = bkm_scale_by_power (&tmp, base, 2);
+ overflow = bkm_scale_by_power (&tmp, xbase, 2);
break;
case 'P': /* peta or pebi */
- overflow = bkm_scale_by_power (&tmp, base, 5);
+ overflow = bkm_scale_by_power (&tmp, xbase, 5);
break;
case 'Q': /* quetta or 2**100 */
- overflow = bkm_scale_by_power (&tmp, base, 10);
+ overflow = bkm_scale_by_power (&tmp, xbase, 10);
break;
case 'R': /* ronna or 2**90 */
- overflow = bkm_scale_by_power (&tmp, base, 9);
+ overflow = bkm_scale_by_power (&tmp, xbase, 9);
break;
case 'T': /* tera or tebi */
case 't': /* 't' is undocumented; for compatibility only */
- overflow = bkm_scale_by_power (&tmp, base, 4);
+ overflow = bkm_scale_by_power (&tmp, xbase, 4);
break;
case 'w':
@@ -224,11 +211,11 @@ __xstrtol (const char *s, char **ptr, int strtol_base,
break;
case 'Y': /* yotta or 2**80 */
- overflow = bkm_scale_by_power (&tmp, base, 8);
+ overflow = bkm_scale_by_power (&tmp, xbase, 8);
break;
case 'Z': /* zetta or 2**70 */
- overflow = bkm_scale_by_power (&tmp, base, 7);
+ overflow = bkm_scale_by_power (&tmp, xbase, 7);
break;
default:
diff --git a/lib/xstrtol.h b/lib/xstrtol.h
index 280c1f3a43..cedff6393e 100644
--- a/lib/xstrtol.h
+++ b/lib/xstrtol.h
@@ -44,8 +44,23 @@ enum strtol_error
typedef enum strtol_error strtol_error;
#endif
+/* Act like the system's strtol (NPTR, ENDPTR, BASE) except:
+ - The TYPE of the result might be something other than long int.
+ - Return strtol_error, and store any result through an additional
+ TYPE *VAL pointer instead of returning the result.
+ - If TYPE is unsigned, reject leading '-'.
+ - 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';
+ these normally stand for powers of 1024, but if VALID_SUFFIXES also
+ includes '0' they can be followed by "B" to stand for the usual
+ SI powers of 1000 (or by "iB" to stand for powers of 1024 as before).
+ Other supported suffixes include 'K' for 1024 or 1000, 'b' for 512,
+ 'c' for 1, and 'w' for 2. */
+
#define _DECLARE_XSTRTOL(name, type) \
- strtol_error name (const char *, char **, int, type *, const char *);
+ strtol_error name (char const *restrict, char **restrict, int, \
+ type *restrict, char const *restrict);
_DECLARE_XSTRTOL (xstrtol, long int)
_DECLARE_XSTRTOL (xstrtoul, unsigned long int)
_DECLARE_XSTRTOL (xstrtoll, long long int)
diff --git a/m4/xstrtol.m4 b/m4/xstrtol.m4
index 20f1de1498..dd6dd17ada 100644
--- a/m4/xstrtol.m4
+++ b/m4/xstrtol.m4
@@ -1,5 +1,5 @@
# xstrtol.m4
-# serial 11
+# serial 12
dnl Copyright (C) 2002-2007, 2009-2024 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -7,5 +7,6 @@ dnl with or without modifications, as long as this notice is preserved.
AC_DEFUN([gl_XSTRTOL],
[
+ AC_REQUIRE([AC_C_RESTRICT])
:
])
diff --git a/modules/xstrtol b/modules/xstrtol
index c9712dee0c..ef49dfc357 100644
--- a/modules/xstrtol
+++ b/modules/xstrtol
@@ -8,7 +8,6 @@ lib/xstrtoul.c
m4/xstrtol.m4
Depends-on:
-assure
intprops
stdckdint
stdint
--
2.43.0
