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

Re: [PATCH v5 3/4] status: give more information during rebase -i

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 1, 2015, 16:36 UTC
Message-ID
<xmqqvbe3al87.fsf@gitster.dls.corp.google.com>
In-Reply-To
<CAPig+cSXzN84C4tOi5oZaK_1UJGcNXywE_-V7CyDQUMXox1Acw@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 21 quoted lines
> I was about to mention the same shortcoming, but you beat me to it.
>
>> Perhaps be more strict and do this instead (without
>> leading strbuf_trim):
>>
>>         if (!get_sha1_hex(split[1]->buf, sha1) &&
>>             !strcmp(split[1]->buf + 40, " ") {
>>                 replace split[1] with "%s " abbrev
>>         }
>
> Isn't it dangerous to assume that you can index 40 characters into
> split[1]? If (for some reason), the user botched the todo line such
> that the SHA1 is no longer a valid hex string, then split[1] will be
> that botched string, which might be shorter than 40 characters. For
> instance, if the user-edited todo line is:
>
>     pick oops nothing
>
> then git-rebase--interactive.sh:transform_todo_ids() will leave "oops"
> as-is, since it can't unabbreviate it, and then this code will place
> "oops" into split[1].

Yeah, that is why get_sha1_hex() is checked before we try to make sure buf[40] has " " in the code snippet you quoted.

Isn't that how && short-cut works?
Previous: Eric SunshineNext: Eric Sunshine
Message 13 of 26 in “More helpful 'git status' during 'rebase -i'”
  1. 0/4 More helpful 'git status' during 'rebase -i'Matthieu Moy, Jun 30, 2015
  2. 1/4 status: factor two rebase-related messages togetherMatthieu Moy, Jun 30, 2015
  3. 2/4 status: differentiate interactive from non-interactive rebasesMatthieu Moy, Jun 30, 2015
  4. 3/4 status: give more information during rebase -iMatthieu Moy, Jun 30, 2015
  5. 4/4 status: add new tests for status during rebase -iMatthieu Moy, Jun 30, 2015
  6. Junio C HamanoJun 30, 2015
  7. Matthieu MoyJul 1, 2015
  8. 1/4 status: factor two rebase-related messages togetherMatthieu Moy, Jul 1, 2015
  9. 2/4 status: differentiate interactive from non-interactive rebasesMatthieu Moy, Jul 1, 2015
  10. 3/4 status: give more information during rebase -iMatthieu Moy, Jul 1, 2015
  11. Junio C HamanoJul 1, 2015
  12. Eric SunshineJul 1, 2015
  13. Junio C HamanoJul 1, 2015
  14. Eric SunshineJul 1, 2015
  15. Matthieu MoyJul 1, 2015
  16. 1/4 status: factor two rebase-related messages togetherMatthieu Moy, Jul 1, 2015
  17. 2/4 status: differentiate interactive from non-interactive rebasesMatthieu Moy, Jul 1, 2015
  18. 3/4 status: give more information during rebase -iMatthieu Moy, Jul 1, 2015
  19. Junio C HamanoJul 7, 2015
  20. Matthieu MoyJul 8, 2015
  21. 4/4 status: add new tests for status during rebase -iMatthieu Moy, Jul 1, 2015
  22. Junio C HamanoJul 1, 2015
  23. Matthieu MoyJul 2, 2015
  24. Junio C HamanoJul 3, 2015
  25. Matthieu MoyJul 2, 2015
  26. 4/4 status: add new tests for status during rebase -iMatthieu Moy, Jul 1, 2015

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.