From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Fri, 04 May 2012 07:24:44 GMT
Subject: Re: [PATCH v2 05/18] refs.c: extract function search_for_subdir()
Message-ID: <4FA3843C.80603@alum.mit.edu>
In-Reply-To: <7vmx5pdlv1.fsf@alter.siamese.dyndns.org>

On 05/03/2012 10:56 PM, Junio C Hamano wrote:
> Junio C Hamano<gitster@pobox.com>  writes:
>
>> Hrm.  The old code used to reset "dir" to NULL before breaking, so the
>> entire function used to return NULL.  Now, it calls search_for_subdir(),
>> which calls search_ref_dir() and gets NULL in entry, and returns NULL.
>>
>> Wouldn't we end up returning the original parameter "dir" instead of NULL
>> in that case?  Would that make a difference?
>
> In other words, isn't something like this necessary?

You are right.  Thanks for catching this.

> Otherwise, wouldn't do_for_each_ref() called for a non-existing "refs/"
> subhierarchy in "base" start from the top-level packed_dir/loose_dir
> returned from find_containing_dir(), and end up running do_for_each_ref_in_dirs()
> with both top-level packed_dir/loose_dir and traversing all of them, only
> to find nothing?
>
>   refs.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index 9f2da16..af5da5f 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -390,8 +390,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,
>   			   refname + dirname.len,
>   			   (slash + 1) - (refname + dirname.len));
>   		subdir = search_for_subdir(dir, dirname.buf, mkdir);
> -		if (!subdir)
> +		if (!subdir) {
> +			dir = NULL;
>   			break;
> +		}
>   		dir = subdir;
>   	}
>


-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/