Hi Collin,

> Bruno Haible via Gnulib discussion list <bug-gnulib@gnu.org> 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  <br...@clisp.org>

        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)




Reply via email to