Re: "git-diff -p :/anything" always segfaults

[Junio C Hamano <junkio@cox.net>]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> 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).