From: Daniel Barkalow <barkalow@iabervon.org>
Date: Wed, 02 Jan 2008 15:53:56 GMT
Subject: Re: input validation in receive-pack
Message-ID: <alpine.LNX.1.00.0801021043220.13593@iabervon.org>
In-Reply-To: <7vhchw3hkz.fsf@gitster.siamese.dyndns.org>

On Tue, 1 Jan 2008, Junio C Hamano wrote:

> @@ -816,9 +821,13 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
>  
>  struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
>  {
> -	if (check_ref_format(ref) == -1)
> +	switch (check_ref_format(ref)) {
> +	case CHECK_REF_FORMAT_ERROR:
> +	case CHECK_REF_FORMAT_WILDCARD:
>  		return NULL;
> -	return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
> +	default:
> +		return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);

It might be wise to make "default" the return NULL case, and list the two 
okay cases explicitly, so it doesn't need to be changed if 
check_ref_format() someday gets additional "okay for some purposes" 
values.

Aside from that, it looks good, except that builtin-send-pack.c and 
fast-import.c should probably use the symbolic constants, too. (All other 
callers only check whether the value is true or not).

	-Daniel
*This .sig left intentionally blank*