From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 02 Jan 2008 22:57:35 GMT
Subject: Re: [PATCH] receive-pack: check object type of sha1 before using them as commits
Message-ID: <7vprwjvns0.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.1.00.0801021418510.3010@woody.linux-foundation.org>

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.