{"thread":{"id":"12182","subject":"[PATCH 04/12] add_one_tree: handle NULL from lookup_tree","startedAt":"2008-02-18T20:47:52Z","lastAt":"2008-02-19T21:31:06Z","messageCount":13,"participants":["Martin Koegler","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":12},"messages":[{"id":"69205","messageId":"12033676833730-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":null,"subject":"[PATCH 01/12] read_object_with_reference: don't read beyond the buffer","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:47:52Z","receivedAt":"2008-02-18T20:47:52Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Signed-off-by: Martin Koegler \n---\n sha1_file.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 4179949..d9da7c8 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1943,7 +1943,8 @@ void *read_object_with_reference(const unsigned char *sha1,\n \t\t}\n \t\tref_length = strlen(ref_type);\n \n-\t\tif (memcmp(buffer, ref_type, ref_length) ||\n+\t\tif (ref_length + 40 > isize ||\n+\t\t memcmp(buffer, ref_type, ref_length) ||\n \t\t get_sha1_hex((char *) buffer + ref_length, actual_sha1)) {\n \t\t\tfree(buffer);\n \t\t\treturn NULL;\n-- \n1.5.4.1.g96b77\n"},{"id":"69207","messageId":"12033676832769-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676833730-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 02/12] get_sha1_oneline: check return value of parse_object","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:47:53Z","receivedAt":"2008-02-18T20:47:53Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Signed-off-by: Martin Koegler \n---\n sha1_name.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex ed3c867..f8506bf 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -620,7 +620,8 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)\n \t\tunsigned long size;\n \n \t\tcommit = pop_most_recent_commit(&list, ONELINE_SEEN);\n-\t\tparse_object(commit->object.sha1);\n+\t\tif (!parse_object(commit->object.sha1))\n+\t\t\tcontinue;\n \t\tif (temp_commit_buffer)\n \t\t\tfree(temp_commit_buffer);\n \t\tif (commit->buffer)\n-- \n1.5.4.1.g96b77\n"},{"id":"69199","messageId":"12033676832231-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676832769-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 03/12] mark_blob/tree_uninteresting: check for NULL","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:47:54Z","receivedAt":"2008-02-18T20:47:54Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"As these functions are directly called with the result\nfrom lookup_tree/blob, they must handle NULL.\n\nSigned-off-by: Martin Koegler \n---\n revision.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 6e85aaa..484e5e7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -46,6 +46,8 @@ void add_object(struct object *obj,\n \n static void mark_blob_uninteresting(struct blob *blob)\n {\n+\tif (!blob)\n+\t\treturn;\n \tif (blob->object.flags & UNINTERESTING)\n \t\treturn;\n \tblob->object.flags |= UNINTERESTING;\n@@ -57,6 +59,8 @@ void mark_tree_uninteresting(struct tree *tree)\n \tstruct name_entry entry;\n \tstruct object *obj = &tree->object;\n \n+\tif (!tree)\n+\t\treturn;\n \tif (obj->flags & UNINTERESTING)\n \t\treturn;\n \tobj->flags |= UNINTERESTING;\n-- \n1.5.4.1.g96b77\n"},{"id":"69198","messageId":"12033676832653-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676832231-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 04/12] add_one_tree: handle NULL from lookup_tree","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:47:55Z","receivedAt":"2008-02-18T20:47:55Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Signed-off-by: Martin Koegler \n---\n reachable.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/reachable.c b/reachable.c\nindex 823e324..937af57 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -150,7 +150,8 @@ static int add_one_reflog(const char *path, const unsigned char *sha1, int flag,\n static void add_one_tree(const unsigned char *sha1, struct rev_info *revs)\n {\n \tstruct tree *tree = lookup_tree(sha1);\n-\tadd_pending_object(revs, &tree->object, \"\");\n+\tif (tree)\n+\t\tadd_pending_object(revs, &tree->object, \"\");\n }\n \n static void add_cache_tree(struct cache_tree *it, struct rev_info *revs)\n-- \n1.5.4.1.g96b77\n"},{"id":"69201","messageId":"12033676833341-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676832653-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 05/12] process_tree/blob: check for NULL","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:47:56Z","receivedAt":"2008-02-18T20:47:56Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"As these functions are directly called with the result\nfrom lookup_tree/blob, they must handle NULL.\n\nSigned-off-by: Martin Koegler \n---\n list-objects.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 4ef58e7..c8b8375 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -18,6 +18,8 @@ static void process_blob(struct rev_info *revs,\n \n \tif (!revs->blob_objects)\n \t\treturn;\n+\tif (!obj)\n+\t\tdie(\"bad blob object\");\n \tif (obj->flags & (UNINTERESTING | SEEN))\n \t\treturn;\n \tobj->flags |= SEEN;\n@@ -69,6 +71,8 @@ static void process_tree(struct rev_info *revs,\n \n \tif (!revs->tree_objects)\n \t\treturn;\n+\tif (!obj)\n+\t\tdie(\"bad tree object\");\n \tif (obj->flags & (UNINTERESTING | SEEN))\n \t\treturn;\n \tif (parse_tree(tree) < 0)\n-- \n1.5.4.1.g96b77\n"},{"id":"69204","messageId":"12033676831961-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676833341-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 06/12] check results of parse_commit in merge_bases","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:47:57Z","receivedAt":"2008-02-18T20:47:57Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"An error is signaled by returning NULL.\n\nSigned-off-by: Martin Koegler \n---\n commit.c | 9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 8b8fb04..70f1266 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -552,8 +552,10 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)\n \t\t */\n \t\treturn commit_list_insert(one, &result);\n \n-\tparse_commit(one);\n-\tparse_commit(two);\n+\tif (parse_commit(one))\n+\t\treturn NULL;\n+\tif (parse_commit(two))\n+\t\treturn NULL;\n \n \tone->object.flags |= PARENT1;\n \ttwo->object.flags |= PARENT2;\n@@ -586,7 +588,8 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)\n \t\t\tparents = parents->next;\n \t\t\tif ((p->object.flags & flags) == flags)\n \t\t\t\tcontinue;\n-\t\t\tparse_commit(p);\n+\t\t\tif (parse_commit(p))\n+\t\t\t\treturn NULL;\n \t\t\tp->object.flags |= flags;\n \t\t\tinsert_by_date(p, &list);\n \t\t}\n-- \n1.5.4.1.g96b77\n"},{"id":"69206","messageId":"1203367683563-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676831961-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 07/12] peel_onion: handle NULL","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:47:58Z","receivedAt":"2008-02-18T20:47:58Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Signed-off-by: Martin Koegler \n---\n sha1_name.c | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex f8506bf..c2805e7 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -494,8 +494,11 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n \t\t\t\treturn error(\"%.*s: expected %s type, but the object dereferences to %s type\",\n \t\t\t\t\t len, name, typename(expected_type),\n \t\t\t\t\t typename(o->type));\n+\t\t\tif (!o)\n+\t\t\t\treturn -1;\n \t\t\tif (!o->parsed)\n-\t\t\t\tparse_object(o->sha1);\n+\t\t\t\tif (!parse_object(o->sha1))\n+\t\t\t\t\treturn -1;\n \t\t}\n \t}\n \treturn 0;\n-- \n1.5.4.1.g96b77\n"},{"id":"69202","messageId":"12033676833893-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"1203367683563-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 08/12] process_tag: handle tag->tagged == NULL","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:47:59Z","receivedAt":"2008-02-18T20:47:59Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Signed-off-by: Martin Koegler \n---\n reachable.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/reachable.c b/reachable.c\nindex 937af57..339be7f 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -79,7 +79,8 @@ static void process_tag(struct tag *tag, struct object_array *p, const char *nam\n \n \tif (parse_tag(tag) < 0)\n \t\tdie(\"bad tag object %s\", sha1_to_hex(obj->sha1));\n-\tadd_object(tag->tagged, p, NULL, name);\n+\tif (tag->tagged)\n+\t\tadd_object(tag->tagged, p, NULL, name);\n }\n \n static void walk_commit_list(struct rev_info *revs)\n-- \n1.5.4.1.g96b77\n"},{"id":"69203","messageId":"12033676842301-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676833893-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 09/12] process_tree/blob: check for NULL","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:48:00Z","receivedAt":"2008-02-18T20:48:00Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"As these functions are directly called with the result\nfrom lookup_tree/blob, they must handle NULL.\n\nSigned-off-by: Martin Koegler \n---\n reachable.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/reachable.c b/reachable.c\nindex 339be7f..3b1c18f 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -15,6 +15,8 @@ static void process_blob(struct blob *blob,\n {\n \tstruct object *obj = &blob->object;\n \n+\tif (!blob)\n+\t\tdie(\"bad blob object\");\n \tif (obj->flags & SEEN)\n \t\treturn;\n \tobj->flags |= SEEN;\n@@ -39,6 +41,8 @@ static void process_tree(struct tree *tree,\n \tstruct name_entry entry;\n \tstruct name_path me;\n \n+\tif (!tree)\n+\t\tdie(\"bad tree object\");\n \tif (obj->flags & SEEN)\n \t\treturn;\n \tobj->flags |= SEEN;\n-- \n1.5.4.1.g96b77\n"},{"id":"69209","messageId":"12033676841373-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676842301-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 10/12] revision.c: handle tag->tagged == NULL","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:48:01Z","receivedAt":"2008-02-18T20:48:01Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Signed-off-by: Martin Koegler \n---\n revision.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 484e5e7..b1aebf8 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -177,6 +177,8 @@ static struct commit *handle_commit(struct rev_info *revs, struct object *object\n \t\tstruct tag *tag = (struct tag *) object;\n \t\tif (revs->tag_objects && !(flags & UNINTERESTING))\n \t\t\tadd_pending_object(revs, object, tag->tag);\n+\t\tif (!tag->tagged)\n+\t\t\tdie(\"bad tag\");\n \t\tobject = parse_object(tag->tagged->sha1);\n \t\tif (!object)\n \t\t\tdie(\"bad object %s\", sha1_to_hex(tag->tagged->sha1));\n@@ -689,6 +691,8 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)\n \t\tit = get_reference(revs, arg, sha1, 0);\n \t\tif (it->type != OBJ_TAG)\n \t\t\tbreak;\n+\t\tif (!((struct tag*)it)->tagged)\n+\t\t\treturn 0;\n \t\thashcpy(sha1, ((struct tag*)it)->tagged->sha1);\n \t}\n \tif (it->type != OBJ_COMMIT)\n-- \n1.5.4.1.g96b77\n"},{"id":"69208","messageId":"12033676841015-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676841373-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 11/12] parse_commit: don't fail, if object is NULL","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:48:02Z","receivedAt":"2008-02-18T20:48:02Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Some codepaths (eg. builtin-rev-parse -> get_merge_bases -> parse_commit)\ncan pass NULL.\n\nSigned-off-by: Martin Koegler \n---\n commit.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 70f1266..5d57450 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -311,6 +311,8 @@ int parse_commit(struct commit *item)\n \tunsigned long size;\n \tint ret;\n \n+\tif (!item)\n+\t\treturn -1;\n \tif (item->object.parsed)\n \t\treturn 0;\n \tbuffer = read_sha1_file(item->object.sha1, &type, &size);\n-- \n1.5.4.1.g96b77\n"},{"id":"69200","messageId":"12033676844141-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"12182","inReplyTo":"12033676841015-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH 12/12] check return value from parse_commit","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-02-18T20:48:03Z","receivedAt":"2008-02-18T20:48:03Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Signed-off-by: Martin Koegler \n---\n commit.c | 3 +--\n shallow.c | 3 ++-\n upload-pack.c | 3 ++-\n 3 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 5d57450..22ce776 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -387,8 +387,7 @@ struct commit *pop_most_recent_commit(struct commit_list **list,\n \n \twhile (parents) {\n \t\tstruct commit *commit = parents->item;\n-\t\tparse_commit(commit);\n-\t\tif (!(commit->object.flags & mark)) {\n+\t\tif (!parse_commit(commit) && !(commit->object.flags & mark)) {\n \t\t\tcommit->object.flags |= mark;\n \t\t\tinsert_by_date(commit, list);\n \t\t}\ndiff --git a/shallow.c b/shallow.c\nindex 212e62b..4d90eda 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -70,7 +70,8 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,\n \t\t\t\tcur_depth = *(int *)commit->util;\n \t\t\t}\n \t\t}\n-\t\tparse_commit(commit);\n+\t\tif (parse_commit(commit))\n+\t\t\tdie(\"invalid commit\");\n \t\tcommit->object.flags |= not_shallow_flag;\n \t\tcur_depth++;\n \t\tfor (p = commit->parents, commit = NULL; p; p = p->next) {\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 53676ee..20d5462 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -534,7 +534,8 @@ static void receive_needs(void)\n \t\t\t\t/* make sure the real parents are parsed */\n \t\t\t\tunregister_shallow(object->sha1);\n \t\t\t\tobject->parsed = 0;\n-\t\t\t\tparse_commit((struct commit *)object);\n+\t\t\t\tif (parse_commit((struct commit *)object))\n+\t\t\t\t\tdie(\"invalid commit\");\n \t\t\t\tparents = ((struct commit *)object)->parents;\n \t\t\t\twhile (parents) {\n \t\t\t\t\tadd_object_array(&parents->item->object,\n-- \n1.5.4.1.g96b77\n"},{"id":"69313","messageId":"7vodaciqg5.fsf@gitster.siamese.dyndns.org","threadId":"12182","inReplyTo":"12033676833341-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH 05/12] process_tree/blob: check for NULL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-19T21:31:06Z","receivedAt":"2008-02-19T21:31:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Koegler writes:\n\n> As these functions are directly called with the result\n> from lookup_tree/blob, they must handle NULL.\n>\n> Signed-off-by: Martin Koegler \n> ---\n> list-objects.c | 4 ++++\n> 1 files changed, 4 insertions(+), 0 deletions(-)\n>\n> diff --git a/list-objects.c b/list-objects.c\n> index 4ef58e7..c8b8375 100644\n> --- a/list-objects.c\n> +++ b/list-objects.c\n> @@ -18,6 +18,8 @@ static void process_blob(struct rev_info *revs,\n> \n> \tif (!revs->blob_objects)\n> \t\treturn;\n> +\tif (!obj)\n> +\t\tdie(\"bad blob object\");\n> \tif (obj->flags & (UNINTERESTING | SEEN))\n> \t\treturn;\n> \tobj->flags |= SEEN;\n> @@ -69,6 +71,8 @@ static void process_tree(struct rev_info *revs,\n> \n> \tif (!revs->tree_objects)\n> \t\treturn;\n> +\tif (!obj)\n> +\t\tdie(\"bad tree object\");\n> \tif (obj->flags & (UNINTERESTING | SEEN))\n> \t\treturn;\n> \tif (parse_tree(tree) < 0)\n\nI think these are in line with process_tree() that barfs like this:\n\n\tif (parse_tree(tree) < 0)\n\t\tdie(\"bad tree object %s\", sha1_to_hex(obj->sha1));\n\nin the existing codepath, but these new die() callsites lose\ninformation.\n\nIt would be nicer if we can report what entry (name) in which\ntree object (sha1) lead to this die(). The same comment applies\nto [09/12].\n\nNevertheless, this is an improvement compared to accessing\nNULL->flags and dying with segv. I'll queue.\n\nThanks.\n"}]}