git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Eric Sunshine" <sunshine@sunshineco.com>,
	"Torsten Bögershausen" <tboegi@web.de>,
	孟子易 <mengziyi540841@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH v2 3/3] shorten_unambiguous_ref(): avoid sscanf()
Date: Thu, 16 Feb 2023 15:36:15 -0800	[thread overview]
Message-ID: <xmqq1qmpnpxs.fsf@gitster.g> (raw)
In-Reply-To: <Y+5nuTbsbuqll5YP@coredump.intra.peff.net> (Jeff King's message of "Thu, 16 Feb 2023 12:28:25 -0500")

Jeff King <peff@peff.net> writes:

> I do kind of wonder if this test is even doing much. It is nice to
> verify the fix (and hopefully somebody with macOS did indeed verify that
> it fails before the fix!). But it does not seem all that likely that we
> are going to regress in this area. I think it's reasonable to include
> it, but if the LC_ALL bit starts creating any portability problems, my
> first instinct would be to drop the test.

That was my thought as well.  I do not see this particular test to
be about protecting against regression.  With the greediness of its
patterns, it is not likely that our future update will reintroduce
use of sscanf(3) and suffer from the same platform bug.  It is about
documenting the historical bug, i.e. the value of the test part of
the patch primarily is that macOS folks can apply it (and revert the
code fix) and demonstrate the presence of a problem.

The refs/remotes/%s/HEAD test does have value---it is to keep us
away from the temptation to go back to the sscanf(3) based solution.
So do the other two to lessor degree, as they cover corner cases
that a similar/rejected implementation would have failed to handle.

Thanks.

  reply	other threads:[~2023-02-16 23:36 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-13  6:38 bug report: symbolic-ref --short command echos the wrong text while use Chinese language 孟子易
2023-02-13 20:18 ` Jeff King
2023-02-13 22:58   ` Eric Sunshine
2023-02-14  1:39     ` Jeff King
2023-02-14  5:15       ` Eric Sunshine
2023-02-14  5:33         ` Eric Sunshine
2023-02-14  5:40           ` Junio C Hamano
2023-02-14  6:05             ` Eric Sunshine
2023-02-14  6:45               ` Junio C Hamano
2023-02-14  6:55                 ` Eric Sunshine
2023-02-14 16:01                   ` Jeff King
2023-02-14 16:29                     ` Eric Sunshine
2023-02-14 17:07                       ` Jeff King
2023-02-14 18:38                         ` [PATCH 0/3] get rid of sscanf() when shortening refs Jeff King
2023-02-14 18:39                           ` [PATCH 1/3] shorten_unambiguous_ref(): avoid integer truncation Jeff King
2023-02-14 18:40                           ` [PATCH 2/3] shorten_unambiguous_ref(): use NUM_REV_PARSE_RULES constant Jeff King
2023-02-14 21:34                             ` Junio C Hamano
2023-02-14 22:23                               ` Jeff King
2023-02-14 18:41                           ` [PATCH 3/3] shorten_unambiguous_ref(): avoid sscanf() Jeff King
2023-02-14 21:48                             ` Junio C Hamano
2023-02-14 22:25                               ` Junio C Hamano
2023-02-14 22:30                               ` Jeff King
2023-02-14 22:34                                 ` Junio C Hamano
2023-02-14 22:40                                   ` Jeff King
2023-02-15  5:10                                     ` Junio C Hamano
2023-02-15 14:30                                       ` Jeff King
2023-02-15 16:41                                         ` Junio C Hamano
2023-02-14 23:20                               ` Eric Sunshine
2023-02-15 15:16                           ` [PATCH v2 0/3] get rid of sscanf() when shortening refs Jeff King
2023-02-15 15:16                             ` [PATCH v2 1/3] shorten_unambiguous_ref(): avoid integer truncation Jeff King
2023-02-15 15:16                             ` [PATCH v2 2/3] shorten_unambiguous_ref(): use NUM_REV_PARSE_RULES constant Jeff King
2023-02-15 15:16                             ` [PATCH v2 3/3] shorten_unambiguous_ref(): avoid sscanf() Jeff King
2023-02-16  5:56                               ` Torsten Bögershausen
2023-02-16  6:16                                 ` Eric Sunshine
2023-02-16 17:21                                   ` Junio C Hamano
2023-02-16 17:28                                     ` Jeff King
2023-02-16 23:36                                       ` Junio C Hamano [this message]
2023-02-16 17:31                                 ` Jeff King
2023-02-17  6:46                                   ` Torsten Bögershausen
2023-02-15 18:00                             ` [PATCH v2 0/3] get rid of sscanf() when shortening refs Junio C Hamano
2023-02-14 16:40                     ` bug report: symbolic-ref --short command echos the wrong text while use Chinese language Junio C Hamano
2023-02-14 17:40                       ` Jeff King
2023-02-15 16:26   ` Torsten Bögershausen
2023-02-15 16:37     ` Eric Sunshine
2023-02-15 17:19       ` Torsten Bögershausen
2023-02-16  6:08         ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq1qmpnpxs.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mengziyi540841@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).