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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 12, 2020, 21:46 UTC
Message-ID: <xmqqwo6kcr83.fsf@gitster.c.googlers.com>
In-Reply-To: <20200412133022.17590-5-worldhello.net@gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

Show 11 quoted lines

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