git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Feb 14, 2023, 23:20 UTC
Message-ID
<CAPig+cQPsrEJ3thaJ1cTkqFaKe34vmrtz_jNLHYTPkBoyMPm8g@mail.gmail.com>
In-Reply-To
<xmqqmt5f535i.fsf@gitster.g>
On Tue, Feb 14, 2023 at 4:48 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 19 quoted lines
> 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.

Previous: Junio C HamanoNext: Jeff King
Message 28 of 46 in “bug report: symbolic-ref --short command echos the wrong text while use Chinese language”
  1. 孟子易Feb 13, 2023
  2. Jeff KingFeb 13, 2023
  3. Eric SunshineFeb 13, 2023
  4. Jeff KingFeb 14, 2023
  5. Eric SunshineFeb 14, 2023
  6. Eric SunshineFeb 14, 2023
  7. Junio C HamanoFeb 14, 2023
  8. Eric SunshineFeb 14, 2023
  9. Junio C HamanoFeb 14, 2023
  10. Eric SunshineFeb 14, 2023
  11. Jeff KingFeb 14, 2023
  12. Eric SunshineFeb 14, 2023
  13. Jeff KingFeb 14, 2023
  14. 0/3 get rid of sscanf() when shortening refsJeff King, Feb 14, 2023
  15. 1/3 shorten_unambiguous_ref(): avoid integer truncationJeff King, Feb 14, 2023
  16. 2/3 shorten_unambiguous_ref(): use NUM_REV_PARSE_RULES constantJeff King, Feb 14, 2023
  17. Junio C HamanoFeb 14, 2023
  18. Jeff KingFeb 14, 2023
  19. 3/3 shorten_unambiguous_ref(): avoid sscanf()Jeff King, Feb 14, 2023
  20. Junio C HamanoFeb 14, 2023
  21. Junio C HamanoFeb 14, 2023
  22. Jeff KingFeb 14, 2023
  23. Junio C HamanoFeb 14, 2023
  24. Jeff KingFeb 14, 2023
  25. Junio C HamanoFeb 15, 2023
  26. Jeff KingFeb 15, 2023
  27. Junio C HamanoFeb 15, 2023
  28. Eric SunshineFeb 14, 2023
  29. 0/3 get rid of sscanf() when shortening refsJeff King, Feb 15, 2023
  30. 1/3 shorten_unambiguous_ref(): avoid integer truncationJeff King, Feb 15, 2023
  31. 2/3 shorten_unambiguous_ref(): use NUM_REV_PARSE_RULES constantJeff King, Feb 15, 2023
  32. 3/3 shorten_unambiguous_ref(): avoid sscanf()Jeff King, Feb 15, 2023
  33. Torsten BögershausenFeb 16, 2023
  34. Eric SunshineFeb 16, 2023
  35. Junio C HamanoFeb 16, 2023
  36. Jeff KingFeb 16, 2023
  37. Junio C HamanoFeb 16, 2023
  38. Jeff KingFeb 16, 2023
  39. Torsten BögershausenFeb 17, 2023
  40. Junio C HamanoFeb 15, 2023
  41. Junio C HamanoFeb 14, 2023
  42. Jeff KingFeb 14, 2023
  43. Torsten BögershausenFeb 15, 2023
  44. Eric SunshineFeb 15, 2023
  45. Torsten BögershausenFeb 15, 2023
  46. Eric SunshineFeb 16, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.