On Mon, Feb 06, 2012 at 12:32:13AM -0800, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> 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.
Yes. Or alternatively, it should just be caught in the strstr() case
below (which would silently ignore it).
> 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.
Yup. See patch 3. :)
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html