Re: [PATCH] blame.c: fix garbled error message

[Jeff King <peff@peff.net>]

On Mon, Jan 12, 2015 at 11:08:26AM -0800, Junio C Hamano wrote:

> Lukas Fleischer <git@cryptocrack.de> writes:
> 
> > The helper functions prepare_final() and prepare_initial() return a
> > pointer to a string that is a member of an object in the revs->pending
> > array. This array is later rebuilt when running prepare_revision_walk()
> > which potentially transforms the pointer target into a bogus string. Fix
> > this by maintaining a copy of the original string.
> >
> > Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
> > ---
> > The bug manifests when running `git blame HEAD^ -- nonexistent.file`.
> 
> Before 1da1e07c (clean up name allocation in prepare_revision_walk,
> 2014-10-15), these strings used to be non-volatile; they were instead
> leaked more or less deliberately.  But these days, these strings are
> cleared, so your patch is absolutely the right thing to do.
> 
> Thanks for catching and fixing.  This fix needs to go to the 2.2.x
> maintenance track.

Yeah, agreed. Sorry for not catching this as part of 1da1e07c.

I did a grep for 'pending.*name' to look at any other potential problem
sites. It looks like blame is the only one that tries to retain a
long-lived pointer to the name.

The other potentially interesting spot is that they are fed to the
object callbacks from traverse_commit_list for tags. However, none of
the callbacks saves it (and it would not make much sense to do so; they
also receive broken-down filenames in the same way, so if they want to
use it at all, they feed it through path_name() first, which makes a
copy).

So I think Lukas's patch fixes everything (and his positioning of the
strdup() calls is right where I would have put them).

-Peff