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

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Nov 5, 2015, 04:00 UTC
Message-ID: <563AD452.2030301@alum.mit.edu>
In-Reply-To: <1446670884.4131.42.camel@twopensource.com>

On 11/04/2015 10:01 PM, David Turner wrote:

Show 14 quoted lines

> 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
mhagger@alum.mit.edu