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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 6, 2012, 08:32 UTC
Message-ID: <7vk4408ir6.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120206081342.GB3966@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 17 quoted lines

> 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