On Tue, Feb 14, 2023 at 4:48 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Jeff King <peff@xxxxxxxx> 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.
