Re: [PATCH] receive-pack: check object type of sha1 before using them as commits

[Junio C Hamano <gitster@pobox.com>]

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

> On Wed, 2 Jan 2008, Martin Koegler wrote:
>>  
>> -		old_commit = (struct commit *)parse_object(old_sha1);
>> -		new_commit = (struct commit *)parse_object(new_sha1);
>> +		old_object = parse_object(old_sha1);
>> +		new_object = parse_object(new_sha1);
>
> I think it would be better to use
>
> 	old_object = lookup_commit_reference(old_sha1);
> 	if (!old_object)
> 		return "bad ref";
> 	new_object = lookup_commit_reference(new_sha1);
> 	if (!new_object)
> 		return "bad ref";
>
> which will write a slightly more useful error message if it's not a commit 
> (ie it will use the "check_commit()" function in commit.c)

Hmmm...  Three points.

 * lookup_commit_reference() is silent if the object pointed at
   by old/new SHA-1 is missing.

 * when storing in refs/heads, we do not want to have committish
   but we do want an actual commit object.

 * returning like that the user cannot tell which ref had the
   error.