Re: [PATCH 2/3] tag: die when listing missing or corrupt objects

Jeff King [off-list ref] writes:

The first case is an indication of a broken or corrupt repo,
and we should notify the user of the error.

The second case is OK to silently ignore; however, the
existing code leaked the buffer returned by read_sha1_file.
...  
 	buf = read_sha1_file(sha1, &type, &size);
-	if (!buf || !size)
+	if (!buf)
+		die_errno("unable to read object %s", sha1_to_hex(sha1));
+	if (!size) {
+		free(buf);
 		return;
+	}
 
 	/* skip header */
 	sp = strstr(buf, "\n\n");

Hmm, a pedant in me says a tag object cannot have zero length, so the
second case is also an indication of a corrupt repository, unless the tag
happens to be a lightweight one that refers directly to a blob object that
is empty.

For that matter, shouldn't we make sure that the type is OBJ_TAG? It might
make sense to allow OBJ_COMMIT (i.e. lightweight tag to a commit) as well,
because the definition of "first N lines" is compatible between tag and
commit for the purpose of the -n option.

For example, in the kernel repository, what would this do, I have to
wonder:

    $ git tag c2.6.12 v2.6.12^{commit}
    $ git tag t2.6.12 v2.6.12^{tree}
    $ git tag -l -n 12 c2.6.12 t2.6.12