Subject: Re: [PATCH v2 3/3] shorten_unambiguous_ref(): avoid sscanf()

From: Junio C Hamano <[email-masked]>
Date: Thu, 16 Feb 2023 09:21:15 -0800

Eric Sunshine <[email-masked]> writes:

>> d) Mention MacOs here ?
> Certainly, a good idea.

Yes.

>> test_expect_success 'symbolic-ref --short handles complex utf8 case' '
>>         name="测试-加-增加-加-增加" &&
>>         git symbolic-ref TEST_SYMREF "refs/heads/$name" &&
>>         # In the real world, we saw problems with this case only under MacOs Ventura
>
> I'm on ancient High Sierra (10.13) using HFS+, so the problem is not
> Ventura-specific. The original bug report did mention Ventura (which
> presumably is using APFS).

I do not think the bug is in sscanf() and not in the code that deals
with filenames in the filesystem.  The symbolic-ref TEST_SYMREF is
implemented by writing the problematic string into a regular file,
and we read it as a stream of bytes---there is no chance for things
like filename normalization the filesystem tries to do to corrupt
it.  So reference to "under macOS Ventuara" I think is about their C
library, not a particular filesystem it uses.

How about a bit more detail on sscanf(), like this?

 t/t1401-symbolic-ref.sh | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git c/t/t1401-symbolic-ref.sh w/t/t1401-symbolic-ref.sh
index be23be30c7..dafcb4d61b 100755
--- c/t/t1401-symbolic-ref.sh
+++ w/t/t1401-symbolic-ref.sh
@@ -192,11 +192,13 @@ test_expect_success 'symbolic-ref pointing at another' '
 test_expect_success 'symbolic-ref --short handles complex utf8 case' '
 	name="测试-加-增加-加-增加" &&
 	git symbolic-ref TEST_SYMREF "refs/heads/$name" &&
-	# In the real world, we saw problems with this case only
-	# when the locale includes UTF-8. Set it here to try to make things as
-	# hard as possible for us to pass, but in practice we should do the
-	# right thing regardless (and of course some platforms may not even
-	# have this locale).
+	# In the real world, we saw this case misbehaved on macOS only
+	# when the locale includes UTF-8, back when "symbolic-ref --short"
+	# used sscanf(3) as part of its implementation.  Set it here to
+	# try to make things as hard as possible for us to pass, but in
+	# practice we should do the right thing regardless (and of course
+	# some platforms may not even have this locale), as we no longer
+	# use platform sscanf(3) there.
 	LC_ALL=en_US.UTF-8 git symbolic-ref --short TEST_SYMREF >actual &&
 	echo "$name" >expect &&
 	test_cmp expect actual