{"thread":{"id":"28136","subject":"[PATCH/RFC] Disallow NULL in struct commit_list","startedAt":"2011-08-17T02:01:18Z","lastAt":"2011-08-17T02:01:18Z","messageCount":1,"participants":["Nguyễn Thái Ngọc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"173656","messageId":"1313546478-8833-1-git-send-email-pclouds@gmail.com","threadId":"28136","inReplyTo":null,"subject":"[PATCH/RFC] Disallow NULL in struct commit_list","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-08-17T02:01:18Z","receivedAt":"2011-08-17T02:01:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It seems to me that in most cases commit_list is supposed to contain\nreal commits. If someone puts a NULL there by accident, it'd be hard to\ntrack down because sigsegv would happen later when the the commit is\nused, not when added.\n\nSo perhaps we could safeguard commit_list_insert(). If a bug tries to\nput NULL in, it'd be caught earlier.\n\nThere is code that add NULL commit on purpose, \"make test\" only catches\none in revert.c, but I may miss other cases and crash system unnecessarily.\n\nNot sure if this patch is worth the trouble. Maybe make it permanent\nresident of next and never graduate to master?\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/merge.c  |    2 +-\n builtin/revert.c |    2 +-\n commit.c         |    3 +++\n commit.h         |    2 ++\n 4 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 325891e..158008d 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -611,7 +611,7 @@ static void write_tree_trivial(unsigned char *sha1)\n \n static const char *merge_argument(struct commit *commit)\n {\n-\tif (commit)\n+\tif (commit && commit != &null_commit)\n \t\treturn sha1_to_hex(commit->object.sha1);\n \telse\n \t\treturn EMPTY_TREE_SHA1_HEX;\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 1f27c63..7d5005e 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -471,7 +471,7 @@ static int do_pick_commit(void)\n \t\t}\n \t\tstrbuf_addstr(&msgbuf, \".\\n\");\n \t} else {\n-\t\tbase = parent;\n+\t\tbase = parent ? parent : &null_commit;\n \t\tbase_label = msg.parent_label;\n \t\tnext = commit;\n \t\tnext_label = msg.label;\ndiff --git a/commit.c b/commit.c\nindex dc22695..604d2f1 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -11,6 +11,8 @@ int save_commit_buffer = 1;\n \n const char *commit_type = \"commit\";\n \n+struct commit null_commit;\n+\n static struct commit *check_commit(struct object *obj,\n \t\t\t\t   const unsigned char *sha1,\n \t\t\t\t   int quiet)\n@@ -363,6 +365,7 @@ int find_commit_subject(const char *commit_buffer, const char **subject)\n struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p)\n {\n \tstruct commit_list *new_list = xmalloc(sizeof(struct commit_list));\n+\tassert(item != NULL);\n \tnew_list->item = item;\n \tnew_list->next = *list_p;\n \t*list_p = new_list;\ndiff --git a/commit.h b/commit.h\nindex 0e36fd0..c9f0743 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -21,6 +21,8 @@ struct commit {\n \tchar *buffer;\n };\n \n+extern struct commit null_commit;\n+\n extern int save_commit_buffer;\n extern const char *commit_type;\n \n-- \n1.7.4.74.g639db\n"}]}