Subject: Re: [PATCH v10 4/8] receive-pack: new config receive.procReceiveRefs

From: Junio C Hamano <[email-masked]>
Date: Sun, 12 Apr 2020 14:46:20 -0700

Jiang Xin <[email-masked]> writes:

> +	if (strcmp(var, "receive.procreceiverefs") == 0) {
> +		char *prefix;
> +		int len;
> +
> +		if (!value)
> +			return config_error_nonbool(var);
> +		prefix = xstrdup(value);
> +		len = strlen(prefix);
> +		while (len && prefix[len - 1] == '/')
> +			prefix[--len] = '\0';
> +		string_list_insert(&proc_receive_refs, prefix);

This smells like a copy of the hidden-refs configuration parsing,
which uses string_list_append(), and its result is used for look-up
routine you split out in [3/8], so we know it is safe to leave the
string_list that holds the prefix unsorted.  Is there a reason why
we want to use _insert(), which spends extra cycles to make sure
that the string list is sorted, instead (e.g. if we were updating
the look-up routine to take advantage of the ordering of the
string-list, perhaps)?  If so, would it make sense to update the
original to match?  If not, copy more faithfully to the original and
use _append() here, too?

Thanks.