{"thread":{"id":"54037","subject":"[PATCH v2] rebase -i: Fix possibly wrong onto hash in todo","startedAt":"2020-08-13T17:43:21Z","lastAt":"2020-08-13T17:43:21Z","messageCount":1,"participants":["Antti Keränen"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"403577","messageId":"20200813174257.ipfnzm2q7cpc7jji@haukka.localdomain","threadId":"54037","inReplyTo":null,"subject":"[PATCH v2] rebase -i: Fix possibly wrong onto hash in todo","fromName":"Antti Keränen","fromEmail":"detegr@rbx.email","sentAt":"2020-08-13T17:42:57Z","receivedAt":"2020-08-13T17:43:21Z","isPatch":true,"sender":{"key":"detegr@rbx.email","avatar":"https://avatars.githubusercontent.com/u/724433?v=4"},"body":"'todo_list_write_to_file' may overwrite the static buffer, originating\nfrom 'find_unique_abbrev', that was used to store the short commit hash\n'c' for \"# Rebase a..b onto c\" message in the todo editor. This is\nbecause the buffer that is returned from 'find_unique_abbrev' is valid\nuntil 4 more calls to `find_unique_abbrev` are made.\n\nAs 'todo_list_write_to_file' calls 'find_unique_abbrev' for each rebased\ncommit, the hash for 'c' is overwritten if there are 4 or more commits\nin the rebase. This behavior has been broken since its introduction.\n\nFix by storing the short onto commit hash in a different buffer that\nremains valid, before calling 'todo_list_write_to_file'.\n\nFound-by: Jussi Keränen <jussike@gmail.com>\nSigned-off-by: Antti Keränen <detegr@rbx.email>\nAcked-by: Alban Gruin <alban.gruin@gmail.com>\n---\n sequencer.c                   | 5 +++--\n t/t3404-rebase-interactive.sh | 6 ++++++\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex fd7701c88a..e2007dbb8c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -5178,13 +5178,14 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla\n \t\t    struct string_list *commands, unsigned autosquash,\n \t\t    struct todo_list *todo_list)\n {\n-\tconst char *shortonto, *todo_file = rebase_path_todo();\n+\tchar shortonto[GIT_MAX_HEXSZ + 1];\n+\tconst char *todo_file = rebase_path_todo();\n \tstruct todo_list new_todo = TODO_LIST_INIT;\n \tstruct strbuf *buf = &todo_list->buf, buf2 = STRBUF_INIT;\n \tstruct object_id oid = onto->object.oid;\n \tint res;\n \n-\tshortonto = find_unique_abbrev(&oid, DEFAULT_ABBREV);\n+\tfind_unique_abbrev_r(shortonto, &oid, DEFAULT_ABBREV);\n \n \tif (buf->len == 0) {\n \t\tstruct todo_item *item = append_new_todo(todo_list);\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 4a7d21f898..1b4fa0843e 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1760,6 +1760,12 @@ test_expect_success 'correct error message for commit --amend after empty pick'\n \ttest_i18ngrep \"middle of a rebase -- cannot amend.\" err\n '\n \n+test_expect_success 'todo has correct onto hash' '\n+\tGIT_SEQUENCE_EDITOR=cat git rebase -i no-conflict-branch~4 no-conflict-branch >actual &&\n+\tonto=$(git rev-parse --short HEAD~4) &&\n+\ttest_i18ngrep \"^# Rebase ..* onto $onto\" actual\n+'\n+\n # This must be the last test in this file\n test_expect_success '$EDITOR and friends are unchanged' '\n \ttest_editor_unchanged\n-- \n2.27.0.395.g84249cff14\n\n"}]}