Re: t3900 failure on macOS, iconv(3) broken?
- From
Torsten Bögershausen <tboegi@web.de>
- Date
- Dec 9, 2025, 16:33 UTC
- Message-ID
- <20251209163356.GA5762@tb-raspi4>
- In-Reply-To
- <53690064-1c98-40e9-8b9a-7ba6bee63703@web.de>
On Mon, Dec 08, 2025 at 11:59:11PM +0100, René Scharfe wrote:
Show 110 quoted lines
> Hi all,
>
> three tests of t3900 fail on macOS 26.1 for me:
>
> not ok 17 - ISO-2022-JP should be shown in UTF-8 now
> not ok 25 - ISO-2022-JP should be shown in UTF-8 now
> not ok 38 - commit --fixup into ISO-2022-JP from UTF-8
>
> Here's the verbose output of the first one:
>
> ----- snip! -----
> expecting success of 3900.17 'ISO-2022-JP should be shown in UTF-8 now':
> compare_with ISO-2022-JP "$TEST_DIRECTORY"/t3900/2-UTF-8.txt
>
> --- /Users/x/src/git/t/t3900/2-UTF-8.txt 2024-10-01 19:43:24.605230684 +0000
> +++ current 2025-12-08 21:52:45.786161909 +0000
> @@ -1,4 +1,4 @@
> はれひほふ
>
> しているのが、いるので。
> -濱浜ほれぷりぽれまびぐりろへ。
> +濱浜ほれぷりぽれまび$0$j$m$X!#
> not ok 17 - ISO-2022-JP should be shown in UTF-8 now
> #
> # compare_with ISO-2022-JP "$TEST_DIRECTORY"/t3900/2-UTF-8.txt
> #
> 1..17
> ----- snap! -----
>
> compare_with runs git show to display a commit message, which in this
> case here was encoded using ISO-2022-JP and is supposed to be reencoded
> to UTF-8, but git show only does that half-way -- the "$0$j$m$X!#" part
> is from the original ISO-2022-JP representation.
>
> That botched conversion is done by utf8.c::reencode_string_iconv(). It
> calls iconv(3) to do the actual work, initially with an output buffer of
> the same size as the input. If the output needs more space the function
> enlarges the buffer and calls iconv(3) again.
>
> iconv(3) won't tell us how much space it needs, but it will report what
> part it already managed to convert, so we can increase the buffer and
> continue from there. ISO-2022-JP has escape codes for switching between
> character sets, so it's a stateful encoding. I guess the iconv(3) on my
> machine forgets the state at the end of part one and then messes up part
> two.
>
> I only noticed now because I used to compile with NO_ICONV for some
> reason.
>
> Is anyone else seeing this breakage as well?
>
> Here's a patch that adds make variable ICONV_BREAKS. It avoids the
> breakage when enabled, by starting over again instead of continuing.
>
> René
>
>
> ---
> Makefile | 6 ++++++
> utf8.c | 13 +++++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 6fc322ff88..cf8a0d3ee9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -181,6 +181,9 @@ include shared.mak
> # byte-order mark (BOM) when writing UTF-16 or UTF-32 and always writes in
> # big-endian format.
> #
> +# Define ICONV_BREAKS if your iconv implementation cannot reliably
> +# break a string into valid substrings.
> +#
> # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound. Define
> # ZLIB_NG if you want to use zlib-ng instead of zlib.
> #
> @@ -1836,6 +1839,9 @@ endif
> ifdef ICONV_OMITS_BOM
> BASIC_CFLAGS += -DICONV_OMITS_BOM
> endif
> +ifdef ICONV_BREAKS
> + BASIC_CFLAGS += -DICONV_BREAKS
> +endif
> ifdef NEEDS_LIBGEN
> EXTLIBS += -lgen
> endif
> diff --git a/utf8.c b/utf8.c
> index 35a0251939..ff0c541fbc 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -515,6 +515,19 @@ char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv,
> out = xrealloc(out, outalloc);
> outpos = out + sofar;
> outsz = outalloc - sofar - 1;
> +#ifdef ICONV_BREAKS
> + /*
> + * If iconv(3) messes up piecemeal conversions
> + * then restore the original pointers, sizes,
> + * and converter state, then retry converting
> + * the full string using the reallocated buffer.
> + */
> + insz += (char *)cp - in;
> + cp = (iconv_ibp)in;
> + outpos = out + bom_len;
> + outsz = outalloc - bom_len - 1;
> + iconv(conv, NULL, NULL, NULL, NULL);
> +#endif
> }
> else {
> *outpos = '\0';I am not sure, if I understand the second call to iconv(NULL....) Here is a slightly different patch. Comments wellcome.
diff --git a/utf8.c b/utf8.c index 35a0251939..b3c1dd2b59 100644 --- a/utf8.c +++ b/utf8.c @@ -486,10 +486,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...) char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv, size_t bom_len, size_t *outsz_p) { - size_t outsz, outalloc; + size_t outsz, outalloc, originsz; char *out, *outpos; iconv_ibp cp; + originsz = insz; outsz = insz; outalloc = st_add(outsz, 1 + bom_len); /* for terminating NUL */ out = xmalloc(outalloc); @@ -515,6 +516,17 @@ char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv, out = xrealloc(out, outalloc); outpos = out + sofar; outsz = outalloc - sofar - 1; +#ifdef __APPLE__ + /* + * Several version of iconv(3) mess up piecemeal conversions. + * Restore the original pointers, sizes, + * and converter state, then retry converting + * the full string using the reallocated buffer. + */ + insz = originsz; + outpos = out + bom_len; + cp = (iconv_ibp)in; +#endif } else { *outpos = '\0';