On Mon, 12 Jan 2015 at 23:55:30, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > Lukas Fleischer <git@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> >> ---
> >> 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.
>
> Sigh, but not so fast.
>
> With the patch applied on top of 1da1e07c (or the result merged to
> 'next' for that matter), I see test breakages in many places "git
> blame" is used, e.g. t7010. Did you run the test suite?
>
No, I didn't.
> This is because it is perfectly normal for prepare_final() to return
> NULL. Unconditionally running xstrdup() would of course fail.
> [...]
Something like
return final_commit_name ? xstrdup(final_commit_name) : NULL;
should work, though, right? Calling free() with a null pointer is fine,
so there is nothing else to do here. You can either amend those two
lines or wait for me to resubmit v2 of the patch tomorrow :)
--
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