Re: [PATCH v2 3/3] shorten_unambiguous_ref(): avoid sscanf()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 16, 2023, 17:21 UTC
- Message-ID
- <xmqqlekxr0fo.fsf@gitster.g>
- In-Reply-To
- <CAPig+cR3fNVnLcfR0K85YgVo3go2p9cbC9ekdmm=orh0WQ+HwQ@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
>> d) Mention MacOs here ? > > Certainly, a good idea.
Yes.
Show 8 quoted lines
>> 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