Hi Tim,

Thanks for explaining your setup and showing their results.

> $ cat 17dc60e624cd6fc3491f9cb002f760d60e66ce8b.txt
> ###########
> ## Review Summary
> 
> This commit correctly fixes a platform-specific bug where uClibc-ng was 
> incorrectly treated as glibc for the UTF-8 locale optimization path. The 
> fix adds `!__UCLIBC__` to all conditions gating UTF-8-specific 
> optimizations on `__GLIBC__ >= 2`.

The fact that your setup did not flag this commit as a regression shows
that Pavel's setup is more useful.

> ## Suggestions
> 
> **lib/mbrtowc.c:114 — NULL-handling condition still uses `__GLIBC__ >= 
> 2` without `!__UCLIBC__`**
> 
> The NULL-handling workaround at line 114 (the `if (s == NULL)` block) 
> still uses `(GNULIB_WCHAR_SINGLE_LOCALE && __GLIBC__ >= 2)` without 
> excluding uClibc-ng. While this is a different concern than the UTF-8 
> optimization (it's about detecting whether the system has a bug where it 
> doesn't return `(size_t) -2` for empty input), it means uClibc-ng will 
> still use the glibc-specific NULL-handling path.
> 
> This is not incorrect per se — the NULL-handling logic is more generic 
> than the UTF-8 optimization — but it would be more consistent to also 
> exclude uClibc-ng here, or explicitly document why uClibc-ng is 
> intentionally included for this specific condition. If uClibc-ng does 
> have the same empty-input bug as glibc, then it's fine; if not, this is 
> unnecessary code.

15 lines of suggestions about a #if, without noticing that this #if condition
is in fact buggy (incomplete): It needs to be enabled on all platforms which
test pwc or n. Including the cases MBRTOWC_NUL_RETVAL_BUG and
MBRTOWC_IN_C_LOCALE_MAYBE_EILSEQ. I'm fixing that through the patch below.


2026-06-06  Bruno Haible  <[email protected]>

        mbrtowc: Fix handling of s==NULL on all possible platforms.
        * lib/mbrtowc.c (rpl_mbrtowc): Enable the s==NULL test also on platforms
        that define only MBRTOWC_NUL_RETVAL_BUG or
        MBRTOWC_IN_C_LOCALE_MAYBE_EILSEQ.

diff --git a/lib/mbrtowc.c b/lib/mbrtowc.c
index 116bb09fbd..8026734689 100644
--- a/lib/mbrtowc.c
+++ b/lib/mbrtowc.c
@@ -110,15 +110,14 @@ is_locale_utf8_cached (void)
 size_t
 rpl_mbrtowc (wchar_t *pwc, const char *s, size_t n, mbstate_t *ps)
 {
-# if (MBRTOWC_RETVAL_BUG || MBRTOWC_EMPTY_INPUT_BUG || 
MBRTOWC_INVALID_UTF8_BUG \
-      || (GNULIB_WCHAR_SINGLE_LOCALE && __GLIBC__ >= 2))
+  /* It's simpler to handle the case s == NULL upfront, than to worry about
+     this case later, before every test of pwc and n.  */
   if (s == NULL)
     {
       pwc = NULL;
       s = "";
       n = 1;
     }
-# endif
 
 # if (MBRTOWC_EMPTY_INPUT_BUG || MBRTOWC_INVALID_UTF8_BUG \
       || (GNULIB_WCHAR_SINGLE_LOCALE && __GLIBC__ >= 2 && !__UCLIBC__))




Reply via email to