On Thu, Jun 04, 2026 at 03:03:06PM +0200, Bruno Haible wrote:
> Pavel Cahyna wrote:
> > Perhaps you could start by providing
> > some commits to analyze (some of them that contain known regressions,
> > some of them that don't - without indicating which one is which) to have
> > a smaller set than e.g. all commits since 2026-01-01?
>
> Makes sense.
>
> Try these commits:
Here are the findings:
> 6aae65332edc58970c0dc287b28c8f1f6250282a
Review: commit 6aae65332e — "quotearg: lessen dependencies"
Verdict: No functional regressions found.
This commit is a straightforward dead-dependency cleanup:
- c-strcaseeq.h removal — The STRCASEEQ macro was eliminated from quotearg.c
in an earlier commit (751e42b0bc, Sep 2025) that rewrote gettext_quote to use
mbrtoc32 instead of locale_charset() comparisons. The #include was left
behind; this commit removes it.
- memcmp dependency removal — memcmp was replaced by memeq in another earlier
commit (6ca831b019, Sep 2025). The module dependency was left behind; this
commit removes it. The memeq dependency remains correctly listed.
Both removals are verified: zero occurrences of STRCASEEQ/CASEEQ/memcmp exist
in the post-commit quotearg.c. The remaining dependency list in
modules/quotearg correctly reflects actual usage.
> 17dc60e624cd6fc3491f9cb002f760d60e66ce8b
Review of commit 17dc60e624cd6fc3491f9cb002f760d60e66ce8b
"mbrtoc32, mbrtowc: do not optimze for uClibc-ng"
Summary: The commit adds `&& !__UCLIBC__` guards to prevent glibc-specific
optimizations from running on uClibc-ng, which defines __GLIBC__ but is not
actually glibc. The mbrtoc32.c changes are correct. However, mbrtowc.c
contains a copy-paste regression.
---
BUG: Copy-paste error in lib/mbrtowc.c n==0 guard (lines 122-123)
The preprocessor guard for the `n == 0` early return was changed from:
# if MBRTOWC_EMPTY_INPUT_BUG || (GNULIB_WCHAR_SINGLE_LOCALE && __GLIBC__ >= 2)
to:
# if (MBRTOC32_EMPTY_INPUT_BUG || _GL_SMALL_WCHAR_T \
|| (GNULIB_WCHAR_SINGLE_LOCALE && __GLIBC__ >= 2 && !__UCLIBC__))
This line was copied verbatim from the corresponding guard in lib/mbrtoc32.c
(line 137), introducing two problems:
1. MBRTOWC_EMPTY_INPUT_BUG was replaced by MBRTOC32_EMPTY_INPUT_BUG. These are
separate macros defined by separate m4 tests (m4/mbrtowc.m4 vs
m4/mbrtoc32.m4), probing different functions. On a system where mbrtowc()
has the empty-input bug but mbrtoc32() does not, the guard will no longer
trigger, and rpl_mbrtowc(pwc, s, 0, ps) will fall through to the buggy
system mbrtowc() instead of returning (size_t) -2.
2. _GL_SMALL_WCHAR_T was added, but this macro is defined only in
lib/uchar.in.h, which mbrtowc.c does not include. This term is dead code
in this compilation unit.
Note the inconsistency within the file itself: line 113 still correctly uses
MBRTOWC_EMPTY_INPUT_BUG in the s==NULL guard, while line 122 was changed to
the wrong macro.
The correct fix should have been:
# if (MBRTOWC_EMPTY_INPUT_BUG \
|| (GNULIB_WCHAR_SINGLE_LOCALE && __GLIBC__ >= 2 && !__UCLIBC__))
Impact: Regression on systems where mbrtowc() has the empty-input bug but
mbrtoc32() does not. On such systems, calling rpl_mbrtowc() with n=0 will
no longer be handled correctly.
Severity: Medium. The bug requires a specific combination of platform
characteristics (mbrtowc empty-input bug present, mbrtoc32 empty-input bug
absent), but on affected systems it silently produces incorrect behavior.
> 53d4558960659ba7c4e9e2757bfb0977a5027fae
Review of commit 53d4558960 — "quotearg: be nicer on macOS etc"
Overall: No functional regressions found. The commit correctly addresses
macOS/Android/musl C locales that use UTF-8 by introducing a
C_LOCALE_MIGHT_BE_MULTIBYTE macro and falling back to <wchar.h> / <wctype.h>
functions.
Findings
--------
1. ChangeLog entry: missing closing paren (Low / cosmetic)
The commit message has "(wch, mbstate, mbs_clear, mbrtowch, chisprint,
wchisprint" followed by "(quotearg_buffer_restyled):" — missing "):" after
"wchisprint". The same typo is in the ChangeLog file. Documentation-only,
no functional impact.
2. Preprocessor indentation inconsistency (Low / cosmetic)
Line 73: "# include <wctype.h>" uses one space after #, but it sits at
nesting depth 2 (inside #else -> # if USE_C_LOCALE). The sibling at line 76,
"# include <uchar.h>", correctly uses two spaces. Should be
"# include <wctype.h>" per the file's convention. No functional impact.
3. Dead (size_t)-3 guards when using mbrtowc (Informational)
When USE_C_LOCALE && C_LOCALE_MIGHT_BE_MULTIBYTE, mbrtowch calls mbrtowc
which never returns (size_t)-3. The #if !GNULIB_MBRTOC32_REGULAR guards that
handle -3 become dead code. The mbsinit loop-control guard is redundant but
correct (mbsinit always returns true after a complete mbrtowc conversion).
No incorrect behavior is possible — this is functionally harmless dead code.
No critical or medium-severity issues. The core logic — preprocessor
partitioning, type selection, unibyte_locale fix, and mbrtowc/iswprint
fallbacks — is correct.
> f6894c646a4f2f544209faf8ecb57ba64d9b8238
## Commit Review: f6894c646a — quotearg: remove SIZE_MAX, INT_BITS
**Verdict: No functional regressions found.**
This commit is a clean minor cleanup that replaces a custom `INT_BITS` macro
(`sizeof(int) * CHAR_BIT`) with the C23-standard `UINT_WIDTH` constant (with
gnulib fallback via the `limits-h` module), and removes a dead `SIZE_MAX`
fallback definition.
### Analysis
1. **INT_BITS → UINT_WIDTH substitution**: Semantically equivalent on all
real-world platforms (both evaluate to 32 for 32-bit int). On hypothetical
padding-bit platforms, `UINT_WIDTH` is actually *more correct* — the old
`INT_BITS` could shift into padding bits (undefined behavior), while
`UINT_WIDTH` stays within value bits. The `quote_these_too` array size is
unchanged on real platforms, and would be safely *larger* on padding-bit
platforms.
2. **SIZE_MAX removal**: Safe. The file already includes `<stdint.h>` (and
depends on the gnulib `stdint-h` module), which defines `SIZE_MAX` on all
supported toolchains. The `#ifndef` guard was dead code.
3. **ABI impact**: None. `struct quoting_options` is opaque — defined only in
`quotearg.c`, forward-declared in `quotearg.h`. External consumers never
allocate or directly access its fields.
4. **Dependency**: The added `limits-h` module dependency in `modules/quotearg`
correctly ensures `UINT_WIDTH` is available. The gnulib `lib/limits.in.h`
provides a fallback definition via `_GL_INTEGER_WIDTH(0, UINT_MAX)`.
5. **Cross-file impact**: `INT_BITS` and the `SIZE_MAX` fallback were
file-local to `quotearg.c`. No other files depended on them.
Regards, Pavel
