{"thread":{"id":"54692","subject":"[PATCH 0/2] Fix a segfault in git log --notes","startedAt":"2020-11-23T03:25:09Z","lastAt":"2020-11-23T18:44:09Z","messageCount":5,"participants":["nate@roosteregg.cc","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"410512","messageId":"20201123032342.24566-1-nate@roosteregg.cc","threadId":"54692","inReplyTo":null,"subject":"[PATCH 0/2] Fix a segfault in git log --notes","fromName":"","fromEmail":"nate@roosteregg.cc","sentAt":"2020-11-23T03:23:40Z","receivedAt":"2020-11-23T03:25:09Z","isPatch":true,"sender":{"key":"nate@roosteregg.cc","avatar":"https://avatars.githubusercontent.com/u/1255937?v=4"},"body":"From: Nate Avers <nate@roosteregg.cc>\n\nHi,\n\nI came across a segfault in git log --notes (also affects diff-tree)\nwhen notes.displayRef is configured with no value (not an empty value).\nIt appears to stem from a missing return statement in\nnotes.c:notes_display_config(). Very small fix. (Not sure how much sense\nit makes to include the test.)\n\nThanks,\nNate\n\nNate Avers (2):\n  notes.c: fix a segfault in notes_display_config()\n  t3301: test proper exit response to no-value notes.displayRef.\n\n notes.c          | 2 +-\n t/t3301-notes.sh | 5 +++++\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\n-- \n2.27.0\n\n"},{"id":"410513","messageId":"20201123032342.24566-2-nate@roosteregg.cc","threadId":"54692","inReplyTo":"20201123032342.24566-1-nate@roosteregg.cc","subject":"[PATCH 1/2] notes.c: fix a segfault in notes_display_config()","fromName":"","fromEmail":"nate@roosteregg.cc","sentAt":"2020-11-23T03:23:41Z","receivedAt":"2020-11-23T03:25:11Z","isPatch":true,"sender":{"key":"nate@roosteregg.cc","avatar":"https://avatars.githubusercontent.com/u/1255937?v=4"},"body":"From: Nate Avers <nate@roosteregg.cc>\n\nIf notes.displayRef is configured with no value[1], control should be\nreturned to the caller when notes.c:notes_display_config() checks if 'v'\nis NULL. Otherwise, both git log --notes and git diff-tree --notes will\nsubsequently segfault when refs.h:has_glob_specials() calls strpbrk()\nwith a NULL first argument.\n\n[1] Examples:\n.git/config:\n[notes]\n\tdisplayRef\n$ git -c notes.displayRef [...]\n\nSigned-off-by: Nate Avers <nate@roosteregg.cc>\n---\n notes.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/notes.c b/notes.c\nindex 564baac64d..d5ac081e76 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -970,7 +970,7 @@ static int notes_display_config(const char *k, const char *v, void *cb)\n \n \tif (*load_refs && !strcmp(k, \"notes.displayref\")) {\n \t\tif (!v)\n-\t\t\tconfig_error_nonbool(k);\n+\t\t\treturn config_error_nonbool(k);\n \t\tstring_list_add_refs_by_glob(&display_notes_refs, v);\n \t}\n \n-- \n2.27.0\n\n"},{"id":"410514","messageId":"20201123032342.24566-3-nate@roosteregg.cc","threadId":"54692","inReplyTo":"20201123032342.24566-1-nate@roosteregg.cc","subject":"[PATCH 2/2] t3301: test proper exit response to no-value notes.displayRef.","fromName":"","fromEmail":"nate@roosteregg.cc","sentAt":"2020-11-23T03:23:42Z","receivedAt":"2020-11-23T03:25:13Z","isPatch":true,"sender":{"key":"nate@roosteregg.cc","avatar":"https://avatars.githubusercontent.com/u/1255937?v=4"},"body":"From: Nate Avers <nate@roosteregg.cc>\n\nSigned-off-by: Nate Avers <nate@roosteregg.cc>\n---\n t/t3301-notes.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 8f43303007..ca60faf480 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -672,6 +672,11 @@ test_expect_success 'notes.displayRef respects order' '\n \ttest_cmp expect-both-reversed actual\n '\n \n+test_expect_success 'notes.displayRef with no value handled gracefully' '\n+\ttest_must_fail git -c notes.displayRef log -0 --notes &&\n+\ttest_must_fail git -c notes.displayRef diff-tree --notes HEAD\n+'\n+\n test_expect_success 'GIT_NOTES_DISPLAY_REF works' '\n \tGIT_NOTES_DISPLAY_REF=refs/notes/commits:refs/notes/other \\\n \t\tgit log -2 >actual &&\n-- \n2.27.0\n\n"},{"id":"410515","messageId":"xmqqzh38o8x8.fsf@gitster.c.googlers.com","threadId":"54692","inReplyTo":"20201123032342.24566-2-nate@roosteregg.cc","subject":"Re: [PATCH 1/2] notes.c: fix a segfault in notes_display_config()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-23T06:59:31Z","receivedAt":"2020-11-23T06:59:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"nate@roosteregg.cc writes:\n\n>  \tif (*load_refs && !strcmp(k, \"notes.displayref\")) {\n>  \t\tif (!v)\n> -\t\t\tconfig_error_nonbool(k);\n> +\t\t\treturn config_error_nonbool(k);\n\n\"git grep config_error_nonbool\" tells us that this is the only\ninstance that ignores the return value from the function and does\nnot cause the caller to die.\n\nLooks good.  Thanks.\n"},{"id":"410545","messageId":"xmqqr1ojoqvi.fsf@gitster.c.googlers.com","threadId":"54692","inReplyTo":"20201123032342.24566-3-nate@roosteregg.cc","subject":"Re: [PATCH 2/2] t3301: test proper exit response to no-value notes.displayRef.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-23T18:44:01Z","receivedAt":"2020-11-23T18:44:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"nate@roosteregg.cc writes:\n\n> From: Nate Avers <nate@roosteregg.cc>\n>\n> Signed-off-by: Nate Avers <nate@roosteregg.cc>\n> ---\n>  t/t3301-notes.sh | 5 +++++\n>  1 file changed, 5 insertions(+)\n>\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index 8f43303007..ca60faf480 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -672,6 +672,11 @@ test_expect_success 'notes.displayRef respects order' '\n>  \ttest_cmp expect-both-reversed actual\n>  '\n>  \n> +test_expect_success 'notes.displayRef with no value handled gracefully' '\n> +\ttest_must_fail git -c notes.displayRef log -0 --notes &&\n> +\ttest_must_fail git -c notes.displayRef diff-tree --notes HEAD\n> +'\n\nLooks good.  I didn't know we can feed the \"no assignment means\ntrue\" configuration from the command line with \"git -c\" ;-)\n\n>  test_expect_success 'GIT_NOTES_DISPLAY_REF works' '\n>  \tGIT_NOTES_DISPLAY_REF=refs/notes/commits:refs/notes/other \\\n>  \t\tgit log -2 >actual &&\n"}]}