Re: [PATCH v3 1/8] refs/files: remove redundant check in split_symref_update()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 5, 2025, 21:20 UTC
- Message-ID
- <xmqqjz93kwcm.fsf@gitster.g>
- In-Reply-To
- <20250305-245-partially-atomic-ref-updates-v3-1-0c64e3052354@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 15 quoted lines
> 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 ...
Show 11 quoted lines
> @@ -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 ...
Show 8 quoted lines
> * 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.