{"thread":{"id":"2319","subject":"[PATCH] Fix segfaults caused by invalid tags","startedAt":"2005-11-02T20:07:52Z","lastAt":"2005-11-02T23:42:32Z","messageCount":9,"participants":["Petr Baudis","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"11025","messageId":"20051102200751.26904.5780.stgit@machine.or.cz","threadId":"2319","inReplyTo":null,"subject":"[PATCH] Fix segfaults caused by invalid tags","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-11-02T20:07:52Z","receivedAt":"2005-11-02T20:07:52Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On many places, we didn't bother checking deref_tag() return value against\nNULL, causing GIT segfaulting e.g. on some old Cogito checkouts containing\nresidual tags from GIT.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n fetch-pack.c  |    2 +-\n server-info.c |    5 +++--\n upload-pack.c |    3 ++-\n 3 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 3df9911..1fae211 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -40,7 +40,7 @@ static int rev_list_insert_ref(const cha\n {\n \tstruct object *o = deref_tag(parse_object(sha1));\n \n-\tif (o->type == commit_type)\n+\tif (o && o->type == commit_type)\n \t\trev_list_push((struct commit *)o, SEEN);\n \n \treturn 0;\ndiff --git a/server-info.c b/server-info.c\nindex ba53591..8e13c94 100644\n--- a/server-info.c\n+++ b/server-info.c\n@@ -14,8 +14,9 @@ static int add_info_ref(const char *path\n \tfprintf(info_ref_fp, \"%s\t%s\\n\", sha1_to_hex(sha1), path);\n \tif (o->type == tag_type) {\n \t\to = deref_tag(o);\n-\t\tfprintf(info_ref_fp, \"%s\t%s^{}\\n\",\n-\t\t\tsha1_to_hex(o->sha1), path);\n+\t\tif (o)\n+\t\t\tfprintf(info_ref_fp, \"%s\t%s^{}\\n\",\n+\t\t\t\tsha1_to_hex(o->sha1), path);\n \t}\n \treturn 0;\n }\ndiff --git a/upload-pack.c b/upload-pack.c\nindex c5eff21..f0b8e63 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -227,7 +227,8 @@ static int send_ref(const char *refname,\n \t}\n \tif (o->type == tag_type) {\n \t\to = deref_tag(o);\n-\t\tpacket_write(1, \"%s %s^{}\\n\", sha1_to_hex(o->sha1), refname);\n+\t\tif (o)\n+\t\t\tpacket_write(1, \"%s %s^{}\\n\", sha1_to_hex(o->sha1), refname);\n \t}\n \treturn 0;\n }\n"},{"id":"11026","messageId":"Pine.LNX.4.63.0511022115250.13746@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2319","inReplyTo":"20051102200751.26904.5780.stgit@machine.or.cz","subject":"Re: [PATCH] Fix segfaults caused by invalid tags","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-02T20:16:13Z","receivedAt":"2005-11-02T20:16:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 2 Nov 2005, Petr Baudis wrote:\n\n> On many places, we didn't bother checking deref_tag() return value against\n> NULL, causing GIT segfaulting e.g. on some old Cogito checkouts containing\n> residual tags from GIT.\n\nShouldn't a warning be issued in every of those cases? I don't know, but \nthose tags pointing nowhere don't seem useful to me.\n\nCiao,\nDscho\n"},{"id":"11028","messageId":"7vwtjqrdz5.fsf@assigned-by-dhcp.cox.net","threadId":"2319","inReplyTo":"Pine.LNX.4.63.0511022115250.13746@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Fix segfaults caused by invalid tags","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-02T20:28:46Z","receivedAt":"2005-11-02T20:28:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Shouldn't a warning be issued in every of those cases? I don't know, but \n> those tags pointing nowhere don't seem useful to me.\n\nYes they should warn.\n\nIt is not just \"don't seem useful\"; it is a corrupt repository.\n"},{"id":"11030","messageId":"20051102204101.GE1431@pasky.or.cz","threadId":"2319","inReplyTo":"Pine.LNX.4.63.0511022115250.13746@wbgn013.biozentrum.uni-wuerzburg.de","subject":"[PATCH] Warn when calling deref_tag() on broken tags","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-11-02T20:41:01Z","receivedAt":"2005-11-02T20:41:01Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Wed, Nov 02, 2005 at 09:16:13PM CET, I got a letter\nwhere Johannes Schindelin <Johannes.Schindelin@gmx.de> told me that...\n> On Wed, 2 Nov 2005, Petr Baudis wrote:\n> \n> > On many places, we didn't bother checking deref_tag() return value against\n> > NULL, causing GIT segfaulting e.g. on some old Cogito checkouts containing\n> > residual tags from GIT.\n> \n> Shouldn't a warning be issued in every of those cases? I don't know, but \n> those tags pointing nowhere don't seem useful to me.\n\nGood idea. Not that I would be excited by the awing elegancy of the\npatch...\n\n---\n\nWhen we hit a broken tag by deref_tag(), warn the user.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n commit.c      |    2 +-\n fetch-pack.c  |    4 ++--\n name-rev.c    |    2 +-\n send-pack.c   |    4 ++--\n server-info.c |    2 +-\n sha1_name.c   |    2 +-\n tag.c         |   12 ++++++++++--\n tag.h         |    2 +-\n upload-pack.c |    2 +-\n 9 files changed, 20 insertions(+), 12 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 8f40318..78aac42 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -55,7 +55,7 @@ static struct commit *check_commit(struc\n struct commit *lookup_commit_reference_gently(const unsigned char *sha1,\n \t\t\t\t\t      int quiet)\n {\n-\tstruct object *obj = deref_tag(parse_object(sha1));\n+\tstruct object *obj = deref_tag(parse_object(sha1), NULL);\n \n \tif (!obj)\n \t\treturn NULL;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 1fae211..14b5918 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -38,7 +38,7 @@ static void rev_list_push(struct commit \n \n static int rev_list_insert_ref(const char *path, const unsigned char *sha1)\n {\n-\tstruct object *o = deref_tag(parse_object(sha1));\n+\tstruct object *o = deref_tag(parse_object(sha1), path);\n \n \tif (o && o->type == commit_type)\n \t\trev_list_push((struct commit *)o, SEEN);\n@@ -317,7 +317,7 @@ static int everything_local(struct ref *\n \t * Don't mark them common yet; the server has to be told so first.\n \t */\n \tfor (ref = *refs; ref; ref = ref->next) {\n-\t\tstruct object *o = deref_tag(lookup_object(ref->old_sha1));\n+\t\tstruct object *o = deref_tag(lookup_object(ref->old_sha1), ref->name);\n \n \t\tif (!o || o->type != commit_type || !(o->flags & COMPLETE))\n \t\t\tcontinue;\ndiff --git a/name-rev.c b/name-rev.c\nindex 21fecdf..8ac2d72 100644\n--- a/name-rev.c\n+++ b/name-rev.c\n@@ -164,7 +164,7 @@ int main(int argc, char **argv)\n \t\t\tcontinue;\n \t\t}\n \n-\t\to = deref_tag(parse_object(sha1));\n+\t\to = deref_tag(parse_object(sha1), *argv);\n \t\tif (!o || o->type != commit_type) {\n \t\t\tfprintf(stderr, \"Could not get commit for %s. Skipping.\\n\",\n \t\t\t\t\t*argv);\ndiff --git a/send-pack.c b/send-pack.c\nindex 9f9a6e7..3ee6794 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -126,12 +126,12 @@ static int ref_newer(const unsigned char\n \t/* Both new and old must be commit-ish and new is descendant of\n \t * old.  Otherwise we require --force.\n \t */\n-\to = deref_tag(parse_object(old_sha1));\n+\to = deref_tag(parse_object(old_sha1), NULL);\n \tif (!o || o->type != commit_type)\n \t\treturn 0;\n \told = (struct commit *) o;\n \n-\to = deref_tag(parse_object(new_sha1));\n+\to = deref_tag(parse_object(new_sha1), NULL);\n \tif (!o || o->type != commit_type)\n \t\treturn 0;\n \tnew = (struct commit *) o;\ndiff --git a/server-info.c b/server-info.c\nindex 8e13c94..23d89fb 100644\n--- a/server-info.c\n+++ b/server-info.c\n@@ -13,7 +13,7 @@ static int add_info_ref(const char *path\n \n \tfprintf(info_ref_fp, \"%s\t%s\\n\", sha1_to_hex(sha1), path);\n \tif (o->type == tag_type) {\n-\t\to = deref_tag(o);\n+\t\to = deref_tag(o, path);\n \t\tif (o)\n \t\t\tfprintf(info_ref_fp, \"%s\t%s^{}\\n\",\n \t\t\t\tsha1_to_hex(o->sha1), path);\ndiff --git a/sha1_name.c b/sha1_name.c\nindex fe409fb..ae4c801 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -349,7 +349,7 @@ static int peel_onion(const char *name, \n \tif (!o)\n \t\treturn -1;\n \tif (!type_string) {\n-\t\to = deref_tag(o);\n+\t\to = deref_tag(o, NULL);\n \t\tif (!o || (!o->parsed && !parse_object(o->sha1)))\n \t\t\treturn -1;\n \t\tmemcpy(sha1, o->sha1, 20);\ndiff --git a/tag.c b/tag.c\nindex b1ab75f..9cd09cf 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -3,10 +3,18 @@\n \n const char *tag_type = \"tag\";\n \n-struct object *deref_tag(struct object *o)\n+/* The refname argument is optional and suitable to use only if\n+ * we use this to resolve an actual tag ref. */\n+struct object *deref_tag(struct object *o, const char *refname)\n {\n+\tstruct object *o2 = o;\n \twhile (o && o->type == tag_type)\n-\t\to = parse_object(((struct tag *)o)->tagged->sha1);\n+\t\to = parse_object(((struct tag *)(o2 = o))->tagged->sha1);\n+\tif (!o)\n+\t\tfprintf(stderr, \"warning: invalid tag %s\\n\",\n+\t\t        refname ? refname\n+\t\t        : (o2 ? sha1_to_hex(o2->sha1)\n+\t\t           : \"<unknown> (this is probably internal GIT error)\"));\n \treturn o;\n }\n \ndiff --git a/tag.h b/tag.h\nindex 36e5324..2b60b45 100644\n--- a/tag.h\n+++ b/tag.h\n@@ -15,6 +15,6 @@ struct tag {\n extern struct tag *lookup_tag(const unsigned char *sha1);\n extern int parse_tag_buffer(struct tag *item, void *data, unsigned long size);\n extern int parse_tag(struct tag *item);\n-extern struct object *deref_tag(struct object *);\n+extern struct object *deref_tag(struct object *, const char *refname);\n \n #endif /* TAG_H */\ndiff --git a/upload-pack.c b/upload-pack.c\nindex f0b8e63..cc5b3e3 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -226,7 +226,7 @@ static int send_ref(const char *refname,\n \t\tnr_our_refs++;\n \t}\n \tif (o->type == tag_type) {\n-\t\to = deref_tag(o);\n+\t\to = deref_tag(o, refname);\n \t\tif (o)\n \t\t\tpacket_write(1, \"%s %s^{}\\n\", sha1_to_hex(o->sha1), refname);\n \t}\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"},{"id":"11031","messageId":"Pine.LNX.4.63.0511022145010.13989@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2319","inReplyTo":"20051102204101.GE1431@pasky.or.cz","subject":"Re: [PATCH] Warn when calling deref_tag() on broken tags","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-02T20:47:46Z","receivedAt":"2005-11-02T20:47:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 2 Nov 2005, Petr Baudis wrote:\n\n> diff --git a/tag.c b/tag.c\n> index b1ab75f..9cd09cf 100644\n> --- a/tag.c\n> +++ b/tag.c\n> @@ -3,10 +3,18 @@\n>  \n>  const char *tag_type = \"tag\";\n>  \n> -struct object *deref_tag(struct object *o)\n> +/* The refname argument is optional and suitable to use only if\n> + * we use this to resolve an actual tag ref. */\n> +struct object *deref_tag(struct object *o, const char *refname)\n\nHow about leaving dereg_tag's signature alone, and warn something like \n\n\t\"You have a broken tag (sha1=%s).\\n\"\n\t\"To find out which name(s) it corresponds to, execute\\n\"\n\t\"\\tgit-name-rev --tags %s\\n\"\n\nWhat do you think?\n\nCiao,\nDscho\n"},{"id":"11032","messageId":"20051102205058.GF1431@pasky.or.cz","threadId":"2319","inReplyTo":"Pine.LNX.4.63.0511022145010.13989@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Warn when calling deref_tag() on broken tags","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-11-02T20:50:58Z","receivedAt":"2005-11-02T20:50:58Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Wed, Nov 02, 2005 at 09:47:46PM CET, I got a letter\nwhere Johannes Schindelin <Johannes.Schindelin@gmx.de> told me that...\n> On Wed, 2 Nov 2005, Petr Baudis wrote:\n> > diff --git a/tag.c b/tag.c\n> > index b1ab75f..9cd09cf 100644\n> > --- a/tag.c\n> > +++ b/tag.c\n> > @@ -3,10 +3,18 @@\n> >  \n> >  const char *tag_type = \"tag\";\n> >  \n> > -struct object *deref_tag(struct object *o)\n> > +/* The refname argument is optional and suitable to use only if\n> > + * we use this to resolve an actual tag ref. */\n> > +struct object *deref_tag(struct object *o, const char *refname)\n> \n> How about leaving dereg_tag's signature alone, and warn something like \n> \n> \t\"You have a broken tag (sha1=%s).\\n\"\n> \t\"To find out which name(s) it corresponds to, execute\\n\"\n> \t\"\\tgit-name-rev --tags %s\\n\"\n> \n> What do you think?\n\nI don't know. For multiple broken tags, or multiple invocations of\nderef_tag() on the same tag, this will fill a lot of screen estate. And\nif we mostly actually have this information, why not show it to the user\nright aay?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"},{"id":"11034","messageId":"7vbr12px3g.fsf@assigned-by-dhcp.cox.net","threadId":"2319","inReplyTo":"20051102204101.GE1431@pasky.or.cz","subject":"Re: [PATCH] Warn when calling deref_tag() on broken tags","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-02T21:18:43Z","receivedAt":"2005-11-02T21:18:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> +struct object *deref_tag(struct object *o, const char *refname)\n>  {\n> +\tstruct object *o2 = o;\n>  \twhile (o && o->type == tag_type)\n> -\t\to = parse_object(((struct tag *)o)->tagged->sha1);\n> +\t\to = parse_object(((struct tag *)(o2 = o))->tagged->sha1);\n> +\tif (!o)\n> +\t\tfprintf(stderr, \"warning: invalid tag %s\\n\",\n> +\t\t        refname ? refname\n> +\t\t        : (o2 ? sha1_to_hex(o2->sha1)\n> +\t\t           : \"<unknown> (this is probably internal GIT error)\"));\n>  \treturn o;\n>  }\n\nI wonder if it would make more sense to keep the root of the\ntraversal for this function (i.e. the original value of o) for\nerror reporting purposes, like this:\n\n        struct object *deref_tag(struct object *o, const char *refname)\n        {\n                struct object *o2 = o;\n                while (o && o->type == tag_type)\n                        o = parse_object(((struct tag *)o)->tagged->sha1);\n                if (!o)\n                        fprintf(stderr, \"warning: invalid tag %s\\n\",\n                                refname ? refname\n                                : (o2 ? sha1_to_hex(o2->sha1)\n                                   : \"<unknown> (this is probably internal GIT error)\"));\n                return o;\n        }\n\nWhat do you think?\n\nOther than that, the patch is OK by me.  Thanks.\n"},{"id":"11048","messageId":"7vhdauocv6.fsf@assigned-by-dhcp.cox.net","threadId":"2319","inReplyTo":"20051102204101.GE1431@pasky.or.cz","subject":"Re: [PATCH] Warn when calling deref_tag() on broken tags","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-02T23:21:01Z","receivedAt":"2005-11-02T23:21:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> Good idea. Not that I would be excited by the awing elegancy of the\n> patch...\n\nIt turns out to be not so good idea.  Some places call deref_tag\njust to see if we can unwrap it but not having the referenced\nobject but having the tag object does not necessarily mean a\ncorrupt repository.\n\nFor example, after a fetch that retrieved and stored a tag\nobject but was interrupted before retrieving and storing the\nobject the tag refers to, we do not update the refs of our end\n(which is correct).  After this, imagine fetching from the same\nremote for the same tag.  fetch-pack.c::everything_local will\nfind that we do have the tag object the remote says it has,\nhoping that we can mark it one of the common refs, if that tag\nis something reachable from our refs (it is not).  The caller of\nthe deref_tag() there is careful not to assume the tag is\ncomplete, but it should not bark -- we should just ignore and\npretend that dangling tag object is something we do not have.\n\nI've done it a slightly differently.  Does this look OK to you?\n\n-- >8 -- cut here -- >8 --\n\nSubject: [PATCH] Be careful when dereferencing tags.\n\nOne caller of deref_tag() was not careful enough to make sure\nwhat deref_tag() returned was not NULL (i.e. we found a tag\nobject that points at an object we do not have).  Fix it, and\nwarn about refs that point at such an incomplete tag where\nneeded.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n commit.c      |    2 +-\n fetch-pack.c  |    7 ++++---\n name-rev.c    |    2 +-\n send-pack.c   |    4 ++--\n server-info.c |    7 ++++---\n sha1_name.c   |    2 +-\n tag.c         |    7 ++++++-\n tag.h         |    2 +-\n upload-pack.c |    2 +-\n 9 files changed, 21 insertions(+), 14 deletions(-)\n\napplies-to: 003cf1e61da5c5fe13ed574ae3630c1ee8b56cae\n30a28f848f666ae6f6641baa2917a8ea15ad3160\ndiff --git a/commit.c b/commit.c\nindex 8f40318..a8c9bfc 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -55,7 +55,7 @@ static struct commit *check_commit(struc\n struct commit *lookup_commit_reference_gently(const unsigned char *sha1,\n \t\t\t\t\t      int quiet)\n {\n-\tstruct object *obj = deref_tag(parse_object(sha1));\n+\tstruct object *obj = deref_tag(parse_object(sha1), NULL, 0);\n \n \tif (!obj)\n \t\treturn NULL;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 3df9911..cb21715 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -38,9 +38,9 @@ static void rev_list_push(struct commit \n \n static int rev_list_insert_ref(const char *path, const unsigned char *sha1)\n {\n-\tstruct object *o = deref_tag(parse_object(sha1));\n+\tstruct object *o = deref_tag(parse_object(sha1), path, 0);\n \n-\tif (o->type == commit_type)\n+\tif (o && o->type == commit_type)\n \t\trev_list_push((struct commit *)o, SEEN);\n \n \treturn 0;\n@@ -317,7 +317,8 @@ static int everything_local(struct ref *\n \t * Don't mark them common yet; the server has to be told so first.\n \t */\n \tfor (ref = *refs; ref; ref = ref->next) {\n-\t\tstruct object *o = deref_tag(lookup_object(ref->old_sha1));\n+\t\tstruct object *o = deref_tag(lookup_object(ref->old_sha1),\n+\t\t\t\t\t     NULL, 0);\n \n \t\tif (!o || o->type != commit_type || !(o->flags & COMPLETE))\n \t\t\tcontinue;\ndiff --git a/name-rev.c b/name-rev.c\nindex 21fecdf..59194f1 100644\n--- a/name-rev.c\n+++ b/name-rev.c\n@@ -164,7 +164,7 @@ int main(int argc, char **argv)\n \t\t\tcontinue;\n \t\t}\n \n-\t\to = deref_tag(parse_object(sha1));\n+\t\to = deref_tag(parse_object(sha1), *argv, 0);\n \t\tif (!o || o->type != commit_type) {\n \t\t\tfprintf(stderr, \"Could not get commit for %s. Skipping.\\n\",\n \t\t\t\t\t*argv);\ndiff --git a/send-pack.c b/send-pack.c\nindex 9f9a6e7..3eeb18f 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -126,12 +126,12 @@ static int ref_newer(const unsigned char\n \t/* Both new and old must be commit-ish and new is descendant of\n \t * old.  Otherwise we require --force.\n \t */\n-\to = deref_tag(parse_object(old_sha1));\n+\to = deref_tag(parse_object(old_sha1), NULL, 0);\n \tif (!o || o->type != commit_type)\n \t\treturn 0;\n \told = (struct commit *) o;\n \n-\to = deref_tag(parse_object(new_sha1));\n+\to = deref_tag(parse_object(new_sha1), NULL, 0);\n \tif (!o || o->type != commit_type)\n \t\treturn 0;\n \tnew = (struct commit *) o;\ndiff --git a/server-info.c b/server-info.c\nindex ba53591..0cba8e1 100644\n--- a/server-info.c\n+++ b/server-info.c\n@@ -13,9 +13,10 @@ static int add_info_ref(const char *path\n \n \tfprintf(info_ref_fp, \"%s\t%s\\n\", sha1_to_hex(sha1), path);\n \tif (o->type == tag_type) {\n-\t\to = deref_tag(o);\n-\t\tfprintf(info_ref_fp, \"%s\t%s^{}\\n\",\n-\t\t\tsha1_to_hex(o->sha1), path);\n+\t\to = deref_tag(o, path, 0);\n+\t\tif (o)\n+\t\t\tfprintf(info_ref_fp, \"%s\t%s^{}\\n\",\n+\t\t\t\tsha1_to_hex(o->sha1), path);\n \t}\n \treturn 0;\n }\ndiff --git a/sha1_name.c b/sha1_name.c\nindex fe409fb..be1755a 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -349,7 +349,7 @@ static int peel_onion(const char *name, \n \tif (!o)\n \t\treturn -1;\n \tif (!type_string) {\n-\t\to = deref_tag(o);\n+\t\to = deref_tag(o, name, sp - name - 2);\n \t\tif (!o || (!o->parsed && !parse_object(o->sha1)))\n \t\t\treturn -1;\n \t\tmemcpy(sha1, o->sha1, 20);\ndiff --git a/tag.c b/tag.c\nindex b1ab75f..e574c4b 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -3,10 +3,15 @@\n \n const char *tag_type = \"tag\";\n \n-struct object *deref_tag(struct object *o)\n+struct object *deref_tag(struct object *o, const char *warn, int warnlen)\n {\n \twhile (o && o->type == tag_type)\n \t\to = parse_object(((struct tag *)o)->tagged->sha1);\n+\tif (!o && warn) {\n+\t\tif (!warnlen)\n+\t\t\twarnlen = strlen(warn);\n+\t\terror(\"missing object referenced by '%.*s'\", warnlen, warn);\n+\t}\n \treturn o;\n }\n \ndiff --git a/tag.h b/tag.h\nindex 36e5324..7a0cb00 100644\n--- a/tag.h\n+++ b/tag.h\n@@ -15,6 +15,6 @@ struct tag {\n extern struct tag *lookup_tag(const unsigned char *sha1);\n extern int parse_tag_buffer(struct tag *item, void *data, unsigned long size);\n extern int parse_tag(struct tag *item);\n-extern struct object *deref_tag(struct object *);\n+extern struct object *deref_tag(struct object *, const char *, int);\n \n #endif /* TAG_H */\ndiff --git a/upload-pack.c b/upload-pack.c\nindex c5eff21..be63132 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -226,7 +226,7 @@ static int send_ref(const char *refname,\n \t\tnr_our_refs++;\n \t}\n \tif (o->type == tag_type) {\n-\t\to = deref_tag(o);\n+\t\to = deref_tag(o, refname, 0);\n \t\tpacket_write(1, \"%s %s^{}\\n\", sha1_to_hex(o->sha1), refname);\n \t}\n \treturn 0;\n---\n0.99.9.GIT\n"},{"id":"11050","messageId":"20051102234232.GG1431@pasky.or.cz","threadId":"2319","inReplyTo":"7vhdauocv6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Warn when calling deref_tag() on broken tags","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-11-02T23:42:32Z","receivedAt":"2005-11-02T23:42:32Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Thu, Nov 03, 2005 at 12:21:01AM CET, I got a letter\nwhere Junio C Hamano <junkio@cox.net> told me that...\n> Petr Baudis <pasky@suse.cz> writes:\n> \n> > Good idea. Not that I would be excited by the awing elegancy of the\n> > patch...\n> \n> It turns out to be not so good idea.  Some places call deref_tag\n> just to see if we can unwrap it but not having the referenced\n> object but having the tag object does not necessarily mean a\n> corrupt repository.\n\nAha, that didn't occur to me initially. Yes, what you did is fine by me\n(not that I would be overly proficient in this part of code).\n\n> diff --git a/tag.c b/tag.c\n> index b1ab75f..e574c4b 100644\n> --- a/tag.c\n> +++ b/tag.c\n> @@ -3,10 +3,15 @@\n>  \n>  const char *tag_type = \"tag\";\n>  \n> -struct object *deref_tag(struct object *o)\n> +struct object *deref_tag(struct object *o, const char *warn, int warnlen)\n>  {\n>  \twhile (o && o->type == tag_type)\n>  \t\to = parse_object(((struct tag *)o)->tagged->sha1);\n> +\tif (!o && warn) {\n> +\t\tif (!warnlen)\n> +\t\t\twarnlen = strlen(warn);\n> +\t\terror(\"missing object referenced by '%.*s'\", warnlen, warn);\n> +\t}\n>  \treturn o;\n>  }\n>  \n\nIt should still be a warning, not an error, though. I'd also mention the\n\"tag\" keyword in the message.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"}]}