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

On Thu, 2015-11-05 at 05:00 +0100, Michael Haggerty wrote:

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

quoted

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.

You're right.  I totally missed that.  But while looking at it, I
noticed that the commit message doesn't look quite right (my fault):

Create new function verify_no_descendants, to hold one of the ref
conflict checks used in verify_refname_available. Multiple backends
will need this function, so move it to the common code.

The function is find_descendant_ref not verify_no_descendants.