From: Petr Baudis Date: Wed, 02 Nov 2005 20:41:01 GMT Subject: [PATCH] Warn when calling deref_tag() on broken tags Message-ID: <20051102204101.GE1431@pasky.or.cz> In-Reply-To: Dear diary, on Wed, Nov 02, 2005 at 09:16:13PM CET, I got a letter where Johannes Schindelin told me that... > On Wed, 2 Nov 2005, Petr Baudis wrote: > > > On many places, we didn't bother checking deref_tag() return value against > > NULL, causing GIT segfaulting e.g. on some old Cogito checkouts containing > > residual tags from GIT. > > Shouldn't a warning be issued in every of those cases? I don't know, but > those tags pointing nowhere don't seem useful to me. Good idea. Not that I would be excited by the awing elegancy of the patch... --- When we hit a broken tag by deref_tag(), warn the user. Signed-off-by: Petr Baudis --- commit.c | 2 +- fetch-pack.c | 4 ++-- name-rev.c | 2 +- send-pack.c | 4 ++-- server-info.c | 2 +- sha1_name.c | 2 +- tag.c | 12 ++++++++++-- tag.h | 2 +- upload-pack.c | 2 +- 9 files changed, 20 insertions(+), 12 deletions(-) diff --git a/commit.c b/commit.c index 8f40318..78aac42 100644 --- a/commit.c +++ b/commit.c @@ -55,7 +55,7 @@ static struct commit *check_commit(struc struct commit *lookup_commit_reference_gently(const unsigned char *sha1, int quiet) { - struct object *obj = deref_tag(parse_object(sha1)); + struct object *obj = deref_tag(parse_object(sha1), NULL); if (!obj) return NULL; diff --git a/fetch-pack.c b/fetch-pack.c index 1fae211..14b5918 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -38,7 +38,7 @@ static void rev_list_push(struct commit static int rev_list_insert_ref(const char *path, const unsigned char *sha1) { - struct object *o = deref_tag(parse_object(sha1)); + struct object *o = deref_tag(parse_object(sha1), path); if (o && o->type == commit_type) rev_list_push((struct commit *)o, SEEN); @@ -317,7 +317,7 @@ static int everything_local(struct ref * * Don't mark them common yet; the server has to be told so first. */ for (ref = *refs; ref; ref = ref->next) { - struct object *o = deref_tag(lookup_object(ref->old_sha1)); + struct object *o = deref_tag(lookup_object(ref->old_sha1), ref->name); if (!o || o->type != commit_type || !(o->flags & COMPLETE)) continue; diff --git a/name-rev.c b/name-rev.c index 21fecdf..8ac2d72 100644 --- a/name-rev.c +++ b/name-rev.c @@ -164,7 +164,7 @@ int main(int argc, char **argv) continue; } - o = deref_tag(parse_object(sha1)); + o = deref_tag(parse_object(sha1), *argv); if (!o || o->type != commit_type) { fprintf(stderr, "Could not get commit for %s. Skipping.\n", *argv); diff --git a/send-pack.c b/send-pack.c index 9f9a6e7..3ee6794 100644 --- a/send-pack.c +++ b/send-pack.c @@ -126,12 +126,12 @@ static int ref_newer(const unsigned char /* Both new and old must be commit-ish and new is descendant of * old. Otherwise we require --force. */ - o = deref_tag(parse_object(old_sha1)); + o = deref_tag(parse_object(old_sha1), NULL); if (!o || o->type != commit_type) return 0; old = (struct commit *) o; - o = deref_tag(parse_object(new_sha1)); + o = deref_tag(parse_object(new_sha1), NULL); if (!o || o->type != commit_type) return 0; new = (struct commit *) o; diff --git a/server-info.c b/server-info.c index 8e13c94..23d89fb 100644 --- a/server-info.c +++ b/server-info.c @@ -13,7 +13,7 @@ static int add_info_ref(const char *path fprintf(info_ref_fp, "%s %s\n", sha1_to_hex(sha1), path); if (o->type == tag_type) { - o = deref_tag(o); + o = deref_tag(o, path); if (o) fprintf(info_ref_fp, "%s %s^{}\n", sha1_to_hex(o->sha1), path); diff --git a/sha1_name.c b/sha1_name.c index fe409fb..ae4c801 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -349,7 +349,7 @@ static int peel_onion(const char *name, if (!o) return -1; if (!type_string) { - o = deref_tag(o); + o = deref_tag(o, NULL); if (!o || (!o->parsed && !parse_object(o->sha1))) return -1; memcpy(sha1, o->sha1, 20); diff --git a/tag.c b/tag.c index b1ab75f..9cd09cf 100644 --- a/tag.c +++ b/tag.c @@ -3,10 +3,18 @@ const char *tag_type = "tag"; -struct object *deref_tag(struct object *o) +/* The refname argument is optional and suitable to use only if + * we use this to resolve an actual tag ref. */ +struct object *deref_tag(struct object *o, const char *refname) { + struct object *o2 = o; while (o && o->type == tag_type) - o = parse_object(((struct tag *)o)->tagged->sha1); + o = parse_object(((struct tag *)(o2 = o))->tagged->sha1); + if (!o) + fprintf(stderr, "warning: invalid tag %s\n", + refname ? refname + : (o2 ? sha1_to_hex(o2->sha1) + : " (this is probably internal GIT error)")); return o; } diff --git a/tag.h b/tag.h index 36e5324..2b60b45 100644 --- a/tag.h +++ b/tag.h @@ -15,6 +15,6 @@ struct tag { extern struct tag *lookup_tag(const unsigned char *sha1); extern int parse_tag_buffer(struct tag *item, void *data, unsigned long size); extern int parse_tag(struct tag *item); -extern struct object *deref_tag(struct object *); +extern struct object *deref_tag(struct object *, const char *refname); #endif /* TAG_H */ diff --git a/upload-pack.c b/upload-pack.c index f0b8e63..cc5b3e3 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -226,7 +226,7 @@ static int send_ref(const char *refname, nr_our_refs++; } if (o->type == tag_type) { - o = deref_tag(o); + o = deref_tag(o, refname); if (o) packet_write(1, "%s %s^{}\n", sha1_to_hex(o->sha1), refname); } -- Petr "Pasky" Baudis Stuff: http://pasky.or.cz/ VI has two modes: the one in which it beeps and the one in which it doesn't.