Re: [PATCH] Warn when calling deref_tag() on broken tags

Petr Baudis [off-list ref] writes:

+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)
+		           : "<unknown> (this is probably internal GIT error)"));
 	return o;
 }

I wonder if it would make more sense to keep the root of the
traversal for this function (i.e. the original value of o) for
error reporting purposes, like this:

        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);
                if (!o)
                        fprintf(stderr, "warning: invalid tag %s\n",
                                refname ? refname
                                : (o2 ? sha1_to_hex(o2->sha1)
                                   : "<unknown> (this is probably internal GIT error)"));
                return o;
        }

What do you think?

Other than that, the patch is OK by me.  Thanks.