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

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

On Tue, 2015-11-03 at 08:40 +0100, Michael Haggerty wrote:

quoted

+ * 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