{"thread":{"id":"46456","subject":"[PATCHv2] blame: fix memory corruption scrambling revision name in error message","startedAt":"2017-07-24T21:16:08Z","lastAt":"2017-07-24T21:41:45Z","messageCount":2,"participants":["SZEDER Gábor","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"325000","messageId":"20170724211550.17224-1-szeder.dev@gmail.com","threadId":"46456","inReplyTo":null,"subject":"[PATCHv2] blame: fix memory corruption scrambling revision name in error message","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-07-24T21:15:50Z","receivedAt":"2017-07-24T21:16:08Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"When attempting to blame a non-existing path, git should show an error\nmessage like this:\n\n  $ git blame e83c51633 -- nonexisting-file\n  fatal: no such path nonexisting-file in e83c51633\n\nSince the recent commit 835c49f7d (blame: rework methods that\ndetermine 'final' commit, 2017-05-24) the revision name is either\nmissing or some scrambled characters are shown instead.  The reason is\nthat the revision name must be duplicated, because it is invalidated\nwhen the pending objects array is cleared in the meantime, but this\ncommit dropped the duplication.\n\nRestore the duplication of the revision name in the affected functions\n(find_single_final() and find_single_initial()).\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n\nUse xstrdup_or_null() in the first hunk.\n\n blame.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/blame.c b/blame.c\nindex 91e26e93e..f575e9cbf 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -1663,7 +1663,7 @@ static struct commit *find_single_final(struct rev_info *revs,\n \t\tname = revs->pending.objects[i].name;\n \t}\n \tif (name_p)\n-\t\t*name_p = name;\n+\t\t*name_p = xstrdup_or_null(name);\n \treturn found;\n }\n \n@@ -1735,7 +1735,7 @@ static struct commit *find_single_initial(struct rev_info *revs,\n \t\tdie(\"No commit to dig up from?\");\n \n \tif (name_p)\n-\t\t*name_p = name;\n+\t\t*name_p = xstrdup(name);\n \treturn found;\n }\n \n@@ -1843,6 +1843,8 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam\n \n \tif (orig)\n \t\t*orig = o;\n+\n+\tfree((char *)final_commit_name);\n }\n \n \n-- \n2.14.0.rc0.88.ge338f4246\n\n"},{"id":"325005","messageId":"xmqq4lu1cy32.fsf@gitster.mtv.corp.google.com","threadId":"46456","inReplyTo":"20170724211550.17224-1-szeder.dev@gmail.com","subject":"Re: [PATCHv2] blame: fix memory corruption scrambling revision name in error message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-24T21:41:05Z","receivedAt":"2017-07-24T21:41:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> When attempting to blame a non-existing path, git should show an error\n> message like this:\n>\n>   $ git blame e83c51633 -- nonexisting-file\n>   fatal: no such path nonexisting-file in e83c51633\n>\n> Since the recent commit 835c49f7d (blame: rework methods that\n> determine 'final' commit, 2017-05-24) the revision name is either\n> missing or some scrambled characters are shown instead.  The reason is\n> that the revision name must be duplicated, because it is invalidated\n> when the pending objects array is cleared in the meantime, but this\n> commit dropped the duplication.\n>\n> Restore the duplication of the revision name in the affected functions\n> (find_single_final() and find_single_initial()).\n>\n> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> ---\n\nThanks.  \n\nI vaguely recall we had the same breakage in the code that was\nalready fixed long time ago; that's a strange coincidence if the\njs/blame-lib topic reintroduced the same bug ;-).\n\nWill queue.\n\n>\n> Use xstrdup_or_null() in the first hunk.\n>\n>  blame.c | 6 ++++--\n>  1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/blame.c b/blame.c\n> index 91e26e93e..f575e9cbf 100644\n> --- a/blame.c\n> +++ b/blame.c\n> @@ -1663,7 +1663,7 @@ static struct commit *find_single_final(struct rev_info *revs,\n>  \t\tname = revs->pending.objects[i].name;\n>  \t}\n>  \tif (name_p)\n> -\t\t*name_p = name;\n> +\t\t*name_p = xstrdup_or_null(name);\n>  \treturn found;\n>  }\n>  \n> @@ -1735,7 +1735,7 @@ static struct commit *find_single_initial(struct rev_info *revs,\n>  \t\tdie(\"No commit to dig up from?\");\n>  \n>  \tif (name_p)\n> -\t\t*name_p = name;\n> +\t\t*name_p = xstrdup(name);\n>  \treturn found;\n>  }\n>  \n> @@ -1843,6 +1843,8 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam\n>  \n>  \tif (orig)\n>  \t\t*orig = o;\n> +\n> +\tfree((char *)final_commit_name);\n>  }\n"}]}