Subject: Re: [PATCH v6 25/25] refs: break out ref conflict checks

From: Michael Haggerty <[email-masked]>
Date: Thu, 05 Nov 2015 05:00:18 +0100

On 11/04/2015 10:01 PM, David Turner wrote:
> On Tue, 2015-11-03 at 08:40 +0100, Michael Haggerty wrote:
>> + * extras and skip must be sorted lists of reference names. Either one
>> + * can be NULL, signifying the empty list.
>> + */
> 
> My version had:
> 
> "skip can be NULL; extras cannot."
> 
> The first thing that function does is:
> string_list_find_insert_index(extras, dirname, 0)
> 
> And that crashes when extras is null.  So I think my version is correct
> here.

We're talking about the function find_descendant_ref(), which was added
in this patch, right? Because the first thing that function does is

+	if (!extras)
+		return NULL;

(This guard was in your version, too.) Also, the callsite doesn't
protect against extras==NULL. So either we're talking about two
different things here, or I disagree with you.

> Other than that, I've reviewed both the patches themselves and the
> overall diff and everything looks good to me.

Thanks!

Michael

-- 
Michael Haggerty
[email-masked]