rebase -i: fix possibly wrong onto hash in todo

'todo_list_write_to_file' may overwrite the static buffer, originating from 'find_unique_abbrev', that was used to store the short commit hash 'c' for "# Rebase a..b onto c" message in the todo editor. This is because the buffer that is returned from 'find_unique_abbrev' is valid until 4 more calls to `find_unique_abbrev` are made. As 'todo_list_write_to_file' calls 'find_unique_abbrev' for each rebased commit, the hash for 'c' is overwritten if there are 4 or more commits in the rebase. This behavior has been broken since its introduction. Fix by storing the short onto commit hash in a different buffer that remains valid, before calling 'todo_list_write_to_file'. Found-by: Jussi Keränen <jussike@gmail.com> Signed-off-by: Antti Keränen <detegr@rbx.email> Acked-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Antti Keränen committed Aug 13, 2020 at 20:42 UTC 5da69c0dacb65cf80fc80b53e652fefad7469bf1

2 files changed +9 -2

sequencer.c

+3 -2

@@ -5178,13 +5178,14 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla

5178 struct string_list *commands, unsigned autosquash,

5179 struct todo_list *todo_list)

5180 {

5181 - const char *shortonto, *todo_file = rebase_path_todo();

5181 + char shortonto[GIT_MAX_HEXSZ + 1];

5182 + const char *todo_file = rebase_path_todo();

5183 struct todo_list new_todo = TODO_LIST_INIT;

5184 struct strbuf *buf = &todo_list->buf, buf2 = STRBUF_INIT;

5185 struct object_id oid = onto->object.oid;

5186 int res;

5187

5187 - shortonto = find_unique_abbrev(&oid, DEFAULT_ABBREV);

5188 + find_unique_abbrev_r(shortonto, &oid, DEFAULT_ABBREV);

5189

5190 if (buf->len == 0) {

5191 struct todo_item *item = append_new_todo(todo_list);

t/t3404-rebase-interactive.sh

@@ -1760,6 +1760,12 @@ test_expect_success 'correct error message for commit --amend after empty pick'

1760 test_i18ngrep "middle of a rebase -- cannot amend." err

1761 '

1762

1763 +test_expect_success 'todo has correct onto hash' '

1764 + GIT_SEQUENCE_EDITOR=cat git rebase -i no-conflict-branch~4 no-conflict-branch >actual &&

1765 + onto=$(git rev-parse --short HEAD~4) &&

1766 + test_i18ngrep "^# Rebase ..* onto $onto" actual

1767 +'

1768 +

1769 # This must be the last test in this file

1770 test_expect_success '$EDITOR and friends are unchanged' '

1771 test_editor_unchanged