Re: t3900 failure on macOS, iconv(3) broken?

["Torsten Bögershausen" <tboegi@web.de>]

On Mon, Dec 08, 2025 at 11:59:11PM +0100, René Scharfe wrote:
> 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';