[PATCH 1/3] deref_tag: handle return value NULL

[PATCH 1/3] deref_tag: handle return value NULL

[Martin Koegler]

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 builtin-show-ref.c |    3 +++
 merge-recursive.c  |    2 ++
 sha1_name.c        |    5 ++++-
 shallow.c          |    2 +-
 upload-pack.c      |    3 ++-
 5 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin-show-ref.c b/builtin-show-ref.c
index 65051d1..a323633 100644
--- a/builtin-show-ref.c
+++ b/builtin-show-ref.c
@@ -86,6 +86,9 @@ match:
 			    sha1_to_hex(sha1));
 		if (obj->type == OBJ_TAG) {
 			obj = deref_tag(obj, refname, 0);
+			if (!obj)
+				die("git-show-ref: bad tag at ref %s (%s)", refname,
+				    sha1_to_hex(sha1));
 			hex = find_unique_abbrev(obj->sha1, abbrev);
 			printf("%s %s^{}\n", hex, refname);
 		}
diff --git a/merge-recursive.c b/merge-recursive.c
index dd52342..55ef76f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1673,6 +1673,8 @@ static struct commit *get_ref(const char *ref)
 	if (get_sha1(ref, sha1))
 		die("Could not resolve ref '%s'", ref);
 	object = deref_tag(parse_object(sha1), ref, strlen(ref));
+	if (!object)
+		return NULL;
 	if (object->type == OBJ_TREE)
 		return make_virtual_commit((struct tree*)object,
 			better_branch_name(ref));
diff --git a/sha1_name.c b/sha1_name.c
index be8489e..ed3c867 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -578,8 +578,11 @@ static int handle_one_ref(const char *path,
 	struct object *object = parse_object(sha1);
 	if (!object)
 		return 0;
-	if (object->type == OBJ_TAG)
+	if (object->type == OBJ_TAG) {
 		object = deref_tag(object, path, strlen(path));
+		if (!object)
+			return 0;
+	}
 	if (object->type != OBJ_COMMIT)
 		return 0;
 	insert_by_date((struct commit *)object, list);
diff --git a/shallow.c b/shallow.c
index dbd9f5a..212e62b 100644
--- a/shallow.c
+++ b/shallow.c
@@ -56,7 +56,7 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,
 			if (i < heads->nr) {
 				commit = (struct commit *)
 					deref_tag(heads->objects[i++].item, NULL, 0);
-				if (commit->object.type != OBJ_COMMIT) {
+				if (!commit || commit->object.type != OBJ_COMMIT) {
 					commit = NULL;
 					continue;
 				}
diff --git a/upload-pack.c b/upload-pack.c
index 7e04311..2307f58 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -575,7 +575,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 	}
 	if (o->type == OBJ_TAG) {
 		o = deref_tag(o, refname, 0);
-		packet_write(1, "%s %s^{}\n", sha1_to_hex(o->sha1), refname);
+		if (o)
+			packet_write(1, "%s %s^{}\n", sha1_to_hex(o->sha1), refname);
 	}
 	return 0;
 }
-- 
1.5.4.1.gaf0ae

[PATCH 2/3] deref_tag: handle tag->tagged = NULL

[Martin Koegler]

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 tag.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/tag.c b/tag.c
index 38bf913..990134f 100644
--- a/tag.c
+++ b/tag.c
@@ -9,7 +9,10 @@ const char *tag_type = "tag";
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
 	while (o && o->type == OBJ_TAG)
-		o = parse_object(((struct tag *)o)->tagged->sha1);
+		if (((struct tag *)o)->tagged)
+			o = parse_object(((struct tag *)o)->tagged->sha1);
+		else
+			o = NULL;
 	if (!o && warn) {
 		if (!warnlen)
 			warnlen = strlen(warn);
-- 
1.5.4.1.gaf0ae

[PATCH 3/3] check return code of prepare_revision_walk

[Martin Koegler]

A failure in prepare_revision_walk can be caused by
a not parseable object.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 builtin-blame.c         |    3 ++-
 builtin-fast-export.c   |    3 ++-
 builtin-fmt-merge-msg.c |    3 ++-
 builtin-log.c           |   12 ++++++++----
 builtin-pack-objects.c  |    3 ++-
 builtin-rev-list.c      |    3 ++-
 builtin-shortlog.c      |    3 ++-
 bundle.c                |    3 ++-
 http-push.c             |    3 ++-
 reachable.c             |    3 ++-
 upload-pack.c           |    3 ++-
 11 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index ee45d8a..59d7237 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2367,7 +2367,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	 * bottom commits we would reach while traversing as
 	 * uninteresting.
 	 */
-	prepare_revision_walk(&revs);
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
 
 	if (is_null_sha1(sb.final->object.sha1)) {
 		char *buf;
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index ef27eee..f741df5 100755
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -383,7 +383,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 
 	get_tags_and_duplicates(&revs.pending, &extra_refs);
 
-	prepare_revision_walk(&revs);
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
 	revs.diffopt.format_callback = show_filemodify;
 	DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
 	while ((commit = get_revision(&revs))) {
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index 6163bd4..ebb3f37 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -187,7 +187,8 @@ static void shortlog(const char *name, unsigned char *sha1,
 	add_pending_object(rev, branch, name);
 	add_pending_object(rev, &head->object, "^HEAD");
 	head->object.flags |= UNINTERESTING;
-	prepare_revision_walk(rev);
+	if (prepare_revision_walk(rev))
+		die("revision walk setup failed");
 	while ((commit = get_revision(rev)) != NULL) {
 		char *oneline, *bol, *eol;
 
diff --git a/builtin-log.c b/builtin-log.c
index 99d69f0..5fea64a 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -197,7 +197,8 @@ static int cmd_log_walk(struct rev_info *rev)
 	if (rev->early_output)
 		setup_early_output(rev);
 
-	prepare_revision_walk(rev);
+	if (prepare_revision_walk(rev))
+		die("revision walk setup failed");
 
 	if (rev->early_output)
 		finish_early_output(rev);
@@ -556,7 +557,8 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha
 	o2->flags ^= UNINTERESTING;
 	add_pending_object(&check_rev, o1, "o1");
 	add_pending_object(&check_rev, o2, "o2");
-	prepare_revision_walk(&check_rev);
+	if (prepare_revision_walk(&check_rev))
+		die("revision walk setup failed");
 
 	while ((commit = get_revision(&check_rev)) != NULL) {
 		/* ignore merges */
@@ -781,7 +783,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (!use_stdout)
 		realstdout = xfdopen(xdup(1), "w");
 
-	prepare_revision_walk(&rev);
+	if (prepare_revision_walk(&rev))
+		die("revision walk setup failed");
 	while ((commit = get_revision(&rev)) != NULL) {
 		/* ignore merges */
 		if (commit->parents && commit->parents->next)
@@ -923,7 +926,8 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 		die("Unknown commit %s", limit);
 
 	/* reverse the list of commits */
-	prepare_revision_walk(&revs);
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
 	while ((commit = get_revision(&revs)) != NULL) {
 		/* ignore merges */
 		if (commit->parents && commit->parents->next)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index acb0555..fe95305 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -2042,7 +2042,8 @@ static void get_object_list(int ac, const char **av)
 			die("bad revision '%s'", line);
 	}
 
-	prepare_revision_walk(&revs);
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
 	mark_edges_uninteresting(revs.commits, &revs, show_edge);
 	traverse_commit_list(&revs, show_commit, show_object);
 
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index de80158..9426081 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -609,7 +609,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (bisect_list)
 		revs.limited = 1;
 
-	prepare_revision_walk(&revs);
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
 	if (revs.tree_objects)
 		mark_edges_uninteresting(revs.commits, &revs, show_edge);
 
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index fa8bc7d..0055a57 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -136,7 +136,8 @@ static void get_from_rev(struct rev_info *rev, struct path_list *list)
 {
 	struct commit *commit;
 
-	prepare_revision_walk(rev);
+	if (prepare_revision_walk(rev))
+		die("revision walk setup failed");
 	while ((commit = get_revision(rev)) != NULL) {
 		const char *author = NULL, *buffer;
 
diff --git a/bundle.c b/bundle.c
index 5c95eca..bd12ec8 100644
--- a/bundle.c
+++ b/bundle.c
@@ -128,7 +128,8 @@ int verify_bundle(struct bundle_header *header, int verbose)
 		add_object_array(e->item, e->name, &refs);
 	}
 
-	prepare_revision_walk(&revs);
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
 
 	i = req_nr;
 	while (i && (commit = get_revision(&revs)))
diff --git a/http-push.c b/http-push.c
index b2b410d..63ff218 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2383,7 +2383,8 @@ int main(int argc, char **argv)
 
 		/* Generate a list of objects that need to be pushed */
 		pushing = 0;
-		prepare_revision_walk(&revs);
+		if (prepare_revision_walk(&revs))
+			die("revision walk setup failed");
 		mark_edges_uninteresting(revs.commits);
 		objects_to_send = get_delta(&revs, ref_lock);
 		finish_all_active_slots();
diff --git a/reachable.c b/reachable.c
index 00f289f..823e324 100644
--- a/reachable.c
+++ b/reachable.c
@@ -215,6 +215,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog)
 	 * Set up the revision walk - this will move all commits
 	 * from the pending list to the commit walking list.
 	 */
-	prepare_revision_walk(revs);
+	if (prepare_revision_walk(revs))
+		die("revision walk setup failed");
 	walk_commit_list(revs);
 }
diff --git a/upload-pack.c b/upload-pack.c
index 2307f58..53676ee 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -129,7 +129,8 @@ static int do_rev_list(int fd, void *create_full_pack)
 		}
 		setup_revisions(0, NULL, &revs, NULL);
 	}
-	prepare_revision_walk(&revs);
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
 	mark_edges_uninteresting(revs.commits, &revs, show_edge);
 	traverse_commit_list(&revs, show_commit, show_object);
 	return 0;
-- 
1.5.4.1.gaf0ae

Re: [PATCH 3/3] check return code of prepare_revision_walk

[Junio C Hamano]

Thanks.

It makes me wonder if it is cleaner to simply die() inside
prepare_revision_walk() if all the callers are to do so
anyway, though.

But we can do that easily later if that turns out to be
desired.

Re: [PATCH 1/3] deref_tag: handle return value NULL

[Junio C Hamano]

Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:

> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>

Looks good, thanks.