Hi Collin,
> Bruno Haible via Gnulib discussion list <[email protected]> writes:
>
> > This patch fixes it, by considering the 'grouping' sequence of numbers.
>
> On FreeBSD 15.0 (cfarm427) I see the following warnings:
>
> In file included from unistdio/u16-u16-vasnprintf.c:57:
> ./vasnprintf.c:5224:77: warning: incompatible pointer types passing
> 'unistring_uint16_t[10]' (aka 'unsigned short[10]') to parameter of type
> 'char *' [-Wincompatible-pointer-types]
> 5224 | thousep =
> thousands_separator_char (thousep_buf);
> |
> ^~~~~~~~~~~
> ./vasnprintf.c:415:32: note: passing argument to parameter 'stackbuf' here
> 415 | thousands_separator_char (char stackbuf[10])
> | ^
> ./vasnprintf.c:5224:49: warning: incompatible pointer types assigning to
> 'const unistring_uint16_t *' (aka 'const unsigned short *') from 'const char
> *' [-Wincompatible-pointer-types]
> 5224 | thousep =
> thousands_separator_char (thousep_buf);
> | ^
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./vasnprintf.c:5225:63: warning: incompatible pointer types passing
> 'const unistring_uint16_t *' (aka 'const unsigned short *') to parameter of
> type 'const char *' [-Wincompatible-pointer-types]
> 5225 | thousep_len = strlen
> (thousep);
> |
> ^~~~~~~
> /usr/include/string.h:102:28: note: passing argument to parameter here
> 102 | size_t strlen(const char *) __pure;
> | ^
>
Thanks a lot for this report! Without you, I would not have noticed,
possibly not even before the next libunistring release.
> Looking at the code, it looks like the comment here is incorrect:
>
> # if WIDE_CHAR_VERSION
> /* DCHAR_T is wchar_t. */
> thousep =
> thousands_separator_wchar (thousep_buf);
> # define thousep_len 1
> # else
> /* DCHAR_T is char. */
> thousep =
> thousands_separator_char (thousep_buf);
> thousep_len = strlen (thousep);
> # endif
>
> Since when compiling unistdio DCHAR_T is not a char.
Yup. Correct.
> Wouldn't it be correct to use:
>
> static const DCHAR_T *
> thousands_separator_char (DCHAR_T *stackbuf[10])
> {
> ...
> }
>
> And copy nl_langinfo (THOUSEP) into stackbuf? And use the unistring
> equivelent of strlen if DCHAR_T != char?
Something like this, yes. Fixed through this patch:
2025-04-25 Bruno Haible <[email protected]>
unistdio/u*-vasnprintf: Fix handling of grouping rule.
Reported by Collin Funk in
<https://lists.gnu.org/archive/html/bug-gnulib/2025-04/msg00180.html>.
* lib/unistdio/u8-vasnprintf.c (DCHAR_STRLEN): New macro.
* lib/unistdio/u8-u8-vasnprintf.c (DCHAR_STRLEN): Likewise.
* lib/unistdio/u16-vasnprintf.c (DCHAR_STRLEN): Likewise.
* lib/unistdio/u16-u16-vasnprintf.c (DCHAR_STRLEN): Likewise.
* lib/unistdio/u32-vasnprintf.c (DCHAR_STRLEN): Likewise.
* lib/unistdio/u32-u32-vasnprintf.c (DCHAR_STRLEN): Likewise.
* lib/unistdio/ulc-vasnprintf.c (DCHAR_STRLEN): Likewise.
* lib/vasnprintf.c (DCHAR_STRLEN): Define fallback.
(thousands_separator_DCHAR): New function.
(VASNPRINTF): Use it when DCHAR_T is uintN_t. Use DCHAR_CPY instead of
memcpy.
diff --git a/lib/unistdio/u16-u16-vasnprintf.c
b/lib/unistdio/u16-u16-vasnprintf.c
index 473e56540c..e38c39067d 100644
--- a/lib/unistdio/u16-u16-vasnprintf.c
+++ b/lib/unistdio/u16-u16-vasnprintf.c
@@ -48,6 +48,7 @@
#define DCHAR_T uint16_t
#define DCHAR_CPY u16_cpy
#define DCHAR_SET u16_set
+#define DCHAR_STRLEN u16_strlen
#define DCHAR_MBSNLEN u16_mbsnlen
#define DCHAR_IS_UINT16_T 1
#define U8_TO_DCHAR u8_to_u16
diff --git a/lib/unistdio/u16-vasnprintf.c b/lib/unistdio/u16-vasnprintf.c
index 8b1b3659dc..be857749d6 100644
--- a/lib/unistdio/u16-vasnprintf.c
+++ b/lib/unistdio/u16-vasnprintf.c
@@ -49,6 +49,7 @@
#define DCHAR_T uint16_t
#define DCHAR_CPY u16_cpy
#define DCHAR_SET u16_set
+#define DCHAR_STRLEN u16_strlen
#define DCHAR_MBSNLEN u16_mbsnlen
#define DCHAR_IS_UINT16_T 1
#define U8_TO_DCHAR u8_to_u16
diff --git a/lib/unistdio/u32-u32-vasnprintf.c
b/lib/unistdio/u32-u32-vasnprintf.c
index a051371647..3bb7dece59 100644
--- a/lib/unistdio/u32-u32-vasnprintf.c
+++ b/lib/unistdio/u32-u32-vasnprintf.c
@@ -48,6 +48,7 @@
#define DCHAR_T uint32_t
#define DCHAR_CPY u32_cpy
#define DCHAR_SET u32_set
+#define DCHAR_STRLEN u32_strlen
#define DCHAR_MBSNLEN u32_mbsnlen
#define DCHAR_IS_UINT32_T 1
#define U8_TO_DCHAR u8_to_u32
diff --git a/lib/unistdio/u32-vasnprintf.c b/lib/unistdio/u32-vasnprintf.c
index 56f3b05c31..c79f53565f 100644
--- a/lib/unistdio/u32-vasnprintf.c
+++ b/lib/unistdio/u32-vasnprintf.c
@@ -49,6 +49,7 @@
#define DCHAR_T uint32_t
#define DCHAR_CPY u32_cpy
#define DCHAR_SET u32_set
+#define DCHAR_STRLEN u32_strlen
#define DCHAR_MBSNLEN u32_mbsnlen
#define DCHAR_IS_UINT32_T 1
#define U8_TO_DCHAR u8_to_u32
diff --git a/lib/unistdio/u8-u8-vasnprintf.c b/lib/unistdio/u8-u8-vasnprintf.c
index f03fb29ef2..ad9dc510f7 100644
--- a/lib/unistdio/u8-u8-vasnprintf.c
+++ b/lib/unistdio/u8-u8-vasnprintf.c
@@ -48,6 +48,7 @@
#define DCHAR_T uint8_t
#define DCHAR_CPY u8_cpy
#define DCHAR_SET u8_set
+#define DCHAR_STRLEN u8_strlen
#define DCHAR_MBSNLEN u8_mbsnlen
#define DCHAR_IS_UINT8_T 1
#define U16_TO_DCHAR u16_to_u8
diff --git a/lib/unistdio/u8-vasnprintf.c b/lib/unistdio/u8-vasnprintf.c
index 61fc236dfa..88e59ae4ad 100644
--- a/lib/unistdio/u8-vasnprintf.c
+++ b/lib/unistdio/u8-vasnprintf.c
@@ -49,6 +49,7 @@
#define DCHAR_T uint8_t
#define DCHAR_CPY u8_cpy
#define DCHAR_SET u8_set
+#define DCHAR_STRLEN u8_strlen
#define DCHAR_MBSNLEN u8_mbsnlen
#define DCHAR_IS_UINT8_T 1
#define U16_TO_DCHAR u16_to_u8
diff --git a/lib/unistdio/ulc-vasnprintf.c b/lib/unistdio/ulc-vasnprintf.c
index 50f3b16aa5..50ae4f7eb7 100644
--- a/lib/unistdio/ulc-vasnprintf.c
+++ b/lib/unistdio/ulc-vasnprintf.c
@@ -49,6 +49,7 @@
#define DCHAR_T char
#define DCHAR_CPY memcpy
#define DCHAR_SET memset
+#define DCHAR_STRLEN strlen
#define DCHAR_MBSNLEN mbsnlen
#define TCHAR_T char
#define DCHAR_IS_TCHAR 1
diff --git a/lib/vasnprintf.c b/lib/vasnprintf.c
index 4004133ae4..f1bfcf3161 100644
--- a/lib/vasnprintf.c
+++ b/lib/vasnprintf.c
@@ -29,6 +29,7 @@
Depends on FCHAR_T.
DCHAR_CPY memcpy like function for DCHAR_T[] arrays.
DCHAR_SET memset like function for DCHAR_T[] arrays.
+ DCHAR_STRLEN strlen like function for DCHAR_T[] arrays.
DCHAR_MBSNLEN mbsnlen like function for DCHAR_T[] arrays.
SNPRINTF The system's snprintf (or similar) function.
This may be either snprintf or swprintf.
@@ -183,6 +184,13 @@
# define TCHAR_T char
# endif
#endif
+#ifndef DCHAR_STRLEN
+# if WIDE_CHAR_VERSION
+# define DCHAR_STRLEN local_wcslen
+# else
+# define DCHAR_STRLEN strlen
+# endif
+#endif
#ifndef DCHAR_MBSNLEN
# if WIDE_CHAR_VERSION
# define DCHAR_MBSNLEN wcsnlen
@@ -439,8 +447,43 @@ thousands_separator_char (char stackbuf[10])
}
# endif
#endif
-/* Maximum number of 'char' in the char[] representation of the thousands
- separator. */
+#if !WIDE_CHAR_VERSION && defined DCHAR_CONV_FROM_ENCODING &&
(NEED_PRINTF_DOUBLE || NEED_PRINTF_LONG_DOUBLE)
+/* Determine the thousands-separator character, as a DCHAR_T[] array,
+ according to the current locale.
+ It is a single Unicode character. */
+# ifndef thousands_separator_DCHAR_defined
+# define thousands_separator_DCHAR_defined 1
+static const DCHAR_T *
+thousands_separator_DCHAR (DCHAR_T stackbuf[10])
+{
+ /* Determine it in a multithread-safe way. */
+ char tmpbuf[10];
+ const char *tmp = thousands_separator_char (tmpbuf);
+ if (*tmp != '\0')
+ {
+ /* Convert it from char[] to DCHAR_T[]. */
+ size_t converted_len = 10;
+ DCHAR_T *converted =
+ DCHAR_CONV_FROM_ENCODING (locale_charset (),
+ iconveh_question_mark,
+ tmp, strlen (tmp) + 1,
+ NULL,
+ stackbuf, &converted_len);
+ if (converted != NULL)
+ {
+ if (converted != stackbuf)
+ /* It should not be so long. */
+ abort ();
+ return stackbuf;
+ }
+ }
+ stackbuf[0] = 0;
+ return stackbuf;
+}
+# endif
+#endif
+/* Maximum number of 'char' in the char[] or DCHAR_T[] representation of the
+ thousands separator. */
#define THOUSEP_CHAR_MAXLEN 3
#if WIDE_CHAR_VERSION && ((NEED_PRINTF_DOUBLE || NEED_PRINTF_LONG_DOUBLE) ||
((NEED_PRINTF_FLAG_ALT_PRECISION_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION ||
NEED_PRINTF_FLAG_GROUPING || NEED_PRINTF_FLAG_GROUPING_INT) && DCHAR_IS_TCHAR))
@@ -5219,6 +5262,10 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
/* DCHAR_T is wchar_t. */
thousep = thousands_separator_wchar
(thousep_buf);
# define thousep_len 1
+# elif defined DCHAR_CONV_FROM_ENCODING
+ /* DCHAR_T is uintN_t. */
+ thousep = thousands_separator_DCHAR
(thousep_buf);
+ thousep_len = DCHAR_STRLEN (thousep);
# else
/* DCHAR_T is char. */
thousep = thousands_separator_char
(thousep_buf);
@@ -5254,7 +5301,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
*--p = thousep[0];
# else
p -= thousep_len;
- memcpy (p, thousep, thousep_len);
+ DCHAR_CPY (p, thousep,
thousep_len);
# endif
insert--;
if (insert == 0)
@@ -5545,6 +5592,10 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
/* DCHAR_T is wchar_t. */
thousep =
thousands_separator_wchar (thousep_buf);
# define thousep_len 1
+# elif defined DCHAR_CONV_FROM_ENCODING
+ /* DCHAR_T is uintN_t. */
+ thousep =
thousands_separator_DCHAR (thousep_buf);
+ thousep_len = DCHAR_STRLEN
(thousep);
# else
/* DCHAR_T is char. */
thousep =
thousands_separator_char (thousep_buf);
@@ -5580,7 +5631,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
*--p = thousep[0];
# else
p -= thousep_len;
- memcpy (p, thousep,
thousep_len);
+ DCHAR_CPY (p, thousep,
thousep_len);
# endif
insert--;
if (insert == 0)
@@ -5819,6 +5870,10 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
/* DCHAR_T is wchar_t. */
thousep = thousands_separator_wchar
(thousep_buf);
# define thousep_len 1
+# elif defined DCHAR_CONV_FROM_ENCODING
+ /* DCHAR_T is uintN_t. */
+ thousep = thousands_separator_DCHAR
(thousep_buf);
+ thousep_len = DCHAR_STRLEN (thousep);
# else
/* DCHAR_T is char. */
thousep = thousands_separator_char
(thousep_buf);
@@ -5854,7 +5909,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
*--p = thousep[0];
# else
p -= thousep_len;
- memcpy (p, thousep, thousep_len);
+ DCHAR_CPY (p, thousep,
thousep_len);
# endif
insert--;
if (insert == 0)
@@ -6153,6 +6208,10 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
/* DCHAR_T is wchar_t. */
thousep =
thousands_separator_wchar (thousep_buf);
# define thousep_len 1
+# elif defined DCHAR_CONV_FROM_ENCODING
+ /* DCHAR_T is uintN_t. */
+ thousep =
thousands_separator_DCHAR (thousep_buf);
+ thousep_len = DCHAR_STRLEN
(thousep);
# else
/* DCHAR_T is char. */
thousep =
thousands_separator_char (thousep_buf);
@@ -6188,7 +6247,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
*--p = thousep[0];
# else
p -= thousep_len;
- memcpy (p, thousep,
thousep_len);
+ DCHAR_CPY (p, thousep,
thousep_len);
# endif
insert--;
if (insert == 0)