Re: "git-diff -p :/anything" always segfaults
- From
Junio C Hamano <junkio@cox.net>
- Date
- Mar 11, 2007, 20:58 UTC
- Message-ID
- <7vslcbv6ol.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <Pine.LNX.4.64.0703111322180.9690@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 25 quoted lines
> On Sun, 11 Mar 2007, Jim Meyering wrote:
>>
>> I like the idea of the new ':/<oneline prefix>' notation, and gave it
>> a try, but all I could get was a segfault. It was dereferencing a NULL
>> commit list. Fix below. With it, this example now works:
>
> The fix is correct, but not complete.
>
>> - while ((commit = pop_most_recent_commit(&list, ONELINE_SEEN))) {
>> + while (list && (commit = pop_most_recent_commit(&list, ONELINE_SEEN))) {
>
> The old code was broken, but the new one isn't much better.
>
> "pop_most_recent_commit()" simply doesn't work that way. It *never*
> returns NULL. So having it as part of a while-loop was buggy to begin
> with, and you fixed the test, but the thing is, it should just look like
>
> while (list) {
> struct commit *commit;
>
> commit = pop_most_recent_commit(&list, ONELINE_SEEN);
> ..
>
> and the "pop_most_recent_commit()" simply shouldn't be part of the
> conditional at all.That's what I did in my tentative commit based on Jim's patch (except "commit" is also used to determine the return value from the function).