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

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Feb 14, 2023 at 4:48 PM Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
> > +     do {
> > +             if (!*rule)
> > +                     BUG("rev-parse rule did not have percent");
> > +             if (*rule == '%')
> > +                     break;
> > +     } while (*refname++ == *rule++);
>
> So, if we have refname="refs/heads/frotz" and rule="refs/%.*s", then
> we'll scan refname and rule to skip over their "refs/" prefix, and
> the next iteration, where post-increment moved the pointers to point
> at 'h' (at the beginning of "heads/frotz") on the refname side and
> '%' on the rule side, we iterate once more, notice *rule is '%', and
> break out of the loop.  We have refname="heads/frotz" and rule="%.*s"
>
> If we have refname="refsXheads/frotz" and rule="refs/%.*s", after
> skipping over "refs", refname points at 'X' while rule points at '/'
> and the loop needs to break.  Both pointers are post-incremented,
> and now we have refname="heads/frotz" and rule="%.*s".

Nice catch. I had sat staring at and worrying about the combined
comparison and post-increment, trying to come up with a failing edge
case but missed this one entirely.

This logic error missed by two people suggests that the patch probably
ought to be accompanied by some new tests.