Re: [RFC/PATCH] tag: add --points-at list option

On Mon, Feb 06, 2012 at 02:04:24AM -0500, Jeff King wrote:

quoted

quoted

Before your patch, a tag whose sha1 could not be read would get its name
printed, and then we would later return without printing anything more.
Now it won't get even the first bit printed.

However, I'm not sure the old behavior wasn't buggy; it would print part
of the line, but never actually print the newline.

If you prefer, I can restore the old behavior just moving the
condition/return back below the refname print; then add "buf" qualifier
to the following fragment and at each intermediate free.

Thinking on it more, your behavior is at least as good as the old. And
it only comes up in a broken repo, anyway, so trying to come up with
some kind of useful outcome is pointless.

Sorry to reverse myself, but I just peeked at the show_reference
function one more time. Unconditionally moving the buffer-reading up
above the "if (!filter->lines)" conditional is not a good idea.

If I do "git tag -l", right now git doesn't have to actually read and
parse each object that has been tagged (lightweight or not). If I use
"git tag -n10", then obviously we do need to read it (and we do). And if
we use your new "--points-at", we also do. But if neither of those
options are in use, it would be nice to avoid the object lookup (it may
not seem like much, but if you have a repo with an insane number of
tags, it can add up).

BTW, writing that helped me notice two bugs in your patch:

  1. You read up to 47 bytes into the buffer without ever checking
     whether size >= 47.

  2. You never check whether the object you read from read_sha1_file is
     actually a tag.

Hmm, the "filter->lines" code for "git tag -n" makes a similar error. It
should probably print nothing for objects that are not tags.

-Peff