Re: [PATCH v3 1/8] refs/files: remove redundant check in split_symref_update()

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> In `split_symref_update()`, there were two checks for duplicate
> refnames:
>
>   - At the start, `string_list_has_string()` ensures the refname is not
>     already in `affected_refnames`, preventing duplicates from being
>     added.
>
>   - After adding the refname, another check verifies whether the newly
>     inserted item has a `util` value.
>
> The second check is unnecessary because the first one guarantees that
> `string_list_insert()` will never encounter a preexisting entry.
>
> Since `item->util` is only used in this context, remove the assignment and
> simplify the surrounding code.

It was a bit unclear what "this context" refers to.  We lost all
assignments to the .util member and that is a safe thing to do
because ...

> @@ -2843,13 +2835,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
>  		if (update->flags & REF_LOG_ONLY)
>  			continue;
>  
> -		item = string_list_append(&affected_refnames, update->refname);
> -		/*
> -		 * We store a pointer to update in item->util, but at
> -		 * the moment we never use the value of this field
> -		 * except to check whether it is non-NULL.
> -		 */
> -		item->util = update;

... of this comment, and the "except to check whether" used to
happen in this code ...

>  	 * be valid as long as affected_refnames is in use, and NOT
>  	 * referent, which might soon be freed by our caller.
>  	 */
> -	item = string_list_insert(affected_refnames, new_update->refname);
> -	if (item->util)
> -		BUG("%s unexpectedly found in affected_refnames",
> -		    new_update->refname);
> -	item->util = new_update;

... which the patch removed.

OK.  Makes perfect sense.

Thanks.