Junio C Hamano <gitster@xxxxxxxxx> wrote:
...
> Your fix is all we can sensibly do. I however think you would
> need similar fix to the same function for other object types, as
> they dereference a potentially NULL pointer the same way.
Good point. Patch below. I wondered if these lookup functions have
always been able to return NULL, since there are many remaining uses
where the possibility of a NULL return value is not handled.
Here are a few that stood out in the output of a quick search:
git-grep -E -A3 'lookup_(commit|tag|blob|tree) *\('
Note: I did not look at cases where the return value is an argument
to some other function.
------------------
builtin-fmt-merge-msg.c:
head = lookup_commit(head_sha1);
...
for (i = 0; i < origins.nr; i++)
shortlog(origins.list[i], origins.payload[i],
head, &rev, limit);
------------------
builtin-read-tree.c: struct tree *subtree = lookup_tree(entry.
sha1);
builtin-read-tree.c- if (!subtree->object.parsed)
------------------
combine-diff.c: struct commit *commit = lookup_commit(sha1);
combine-diff.c- struct commit_list *parents;
------------------
http-push.c: struct commit *head = lookup_commit(head_sha1);
http-push.c: struct commit *branch = lookup_commit(branch_sha1);
http-push.c- struct commit_list *merge_bases = get_merge_bases(head, branch, 1);
------------------
reachable.c: struct tree *tree = lookup_tree(sha1);
reachable.c- add_pending_object(revs, &tree->object, "");
------------------
tag.c: item->tagged = &lookup_blob(sha1)->object;
tag.c- } else if (!strcmp(type, tree_type)) {
tag.c: item->tagged = &lookup_tree(sha1)->object;
tag.c- } else if (!strcmp(type, commit_type)) {
tag.c: item->tagged = &lookup_commit(sha1)->object;
tag.c- } else if (!strcmp(type, tag_type)) {
tag.c: item->tagged = &lookup_tag(sha1)->object;
tag.c- } else {
--------------------------
unpack-trees.c: struct tree *tree = lookup_tree(posns[i]->sha1);
...
unpack-trees.c- parse_tree(tree);
Here's the revised patch:
==================================================================
>From 94152217e8e57d3932b4ba6f7ee014da1f4346d3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Fri, 21 Dec 2007 11:56:32 +0100
Subject: [PATCH] Don't dereference NULL upon lookup failure.
Signed-off-by: Jim Meyering <meyering@xxxxxxxxxx>
---
object.c | 42 +++++++++++++++++++++++++++++-------------
1 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/object.c b/object.c
index 16793d9..9945b25 100644
--- a/object.c
+++ b/object.c
@@ -138,27 +138,43 @@ struct object *parse_object_buffer(const unsigned char *sha1, enum object_type t
if (type == OBJ_BLOB) {
struct blob *blob = lookup_blob(sha1);
- parse_blob_buffer(blob, buffer, size);
- obj = &blob->object;
+ if (!blob)
+ obj = NULL;
+ else {
+ parse_blob_buffer(blob, buffer, size);
+ obj = &blob->object;
+ }
} else if (type == OBJ_TREE) {
struct tree *tree = lookup_tree(sha1);
- obj = &tree->object;
- if (!tree->object.parsed) {
- parse_tree_buffer(tree, buffer, size);
- eaten = 1;
+ if (!tree)
+ obj = NULL;
+ else {
+ obj = &tree->object;
+ if (!tree->object.parsed) {
+ parse_tree_buffer(tree, buffer, size);
+ eaten = 1;
+ }
}
} else if (type == OBJ_COMMIT) {
struct commit *commit = lookup_commit(sha1);
- parse_commit_buffer(commit, buffer, size);
- if (!commit->buffer) {
- commit->buffer = buffer;
- eaten = 1;
+ if (!commit)
+ obj = NULL;
+ else {
+ parse_commit_buffer(commit, buffer, size);
+ if (!commit->buffer) {
+ commit->buffer = buffer;
+ eaten = 1;
+ }
+ obj = &commit->object;
}
- obj = &commit->object;
} else if (type == OBJ_TAG) {
struct tag *tag = lookup_tag(sha1);
- parse_tag_buffer(tag, buffer, size);
- obj = &tag->object;
+ if (!tag)
+ obj = NULL;
+ else {
+ parse_tag_buffer(tag, buffer, size);
+ obj = &tag->object;
+ }
} else {
warning("object %s has unknown type id %d\n", sha1_to_hex(sha1), type);
obj = NULL;
--
1.5.4.rc1.16.g60f3b
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
