git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.
Previous: Karthik NayakNext: Karthik Nayak
Message 3 of 59 in “refs: introduce support for partial reference transactions”
  1. 0/8 refs: introduce support for partial reference transactionsKarthik Nayak, Mar 5, 2025
  2. 1/8 refs/files: remove redundant check in split_symref_update()Karthik Nayak, Mar 5, 2025
  3. Junio C HamanoMar 5, 2025
  4. Karthik NayakMar 6, 2025
  5. 3/8 refs/files: remove duplicate duplicates checkKarthik Nayak, Mar 5, 2025
  6. 2/8 refs: move duplicate refname update check to generic layerKarthik Nayak, Mar 5, 2025
  7. Junio C HamanoMar 5, 2025
  8. Karthik NayakMar 6, 2025
  9. 4/8 refs/reftable: extract code from the transaction preparationKarthik Nayak, Mar 5, 2025
  10. 5/8 refs: introduce enum-based transaction error typesKarthik Nayak, Mar 5, 2025
  11. 6/8 refs: implement partial reference transaction supportKarthik Nayak, Mar 5, 2025
  12. Jeff KingMar 7, 2025
  13. Junio C HamanoMar 7, 2025
  14. Junio C HamanoMar 7, 2025
  15. Karthik NayakMar 7, 2025
  16. config.mak.dev: enable -Wunreachable-codeJeff King, Mar 7, 2025
  17. Junio C HamanoMar 7, 2025
  18. Jeff KingMar 8, 2025
  19. Junio C HamanoMar 10, 2025
  20. Jeff KingMar 10, 2025
  21. Junio C HamanoMar 10, 2025
  22. Jeff KingMar 14, 2025
  23. Jeff KingMar 14, 2025
  24. Junio C HamanoMar 14, 2025
  25. Junio C HamanoMar 14, 2025
  26. Patrick SteinhardtMar 14, 2025
  27. Jeff KingMar 14, 2025
  28. Junio C HamanoMar 14, 2025
  29. Junio C HamanoMar 14, 2025
  30. Mike HommeyJun 3, 2025
  31. Junio C HamanoJun 3, 2025
  32. Mike HommeyJun 3, 2025
  33. Mike HommeyJun 3, 2025
  34. 0/3 -Wunreachable-codeJunio C Hamano, Mar 14, 2025
  35. 1/3 config.mak.dev: enable -Wunreachable-codeJunio C Hamano, Mar 14, 2025
  36. 2/3 run-command: use errno to check for sigfillset() errorJunio C Hamano, Mar 14, 2025
  37. Taylor BlauMar 17, 2025
  38. Junio C HamanoMar 17, 2025
  39. Junio C HamanoMar 18, 2025
  40. 3/3 git-compat-util: add NOT_A_CONST macro and use it in atfork_prepare()Junio C Hamano, Mar 14, 2025
  41. Junio C HamanoMar 14, 2025
  42. Jeff KingMar 17, 2025
  43. 0/3 -Wunreachable-codeJunio C Hamano, Mar 17, 2025
  44. 1/3 run-command: use errno to check for sigfillset() errorJunio C Hamano, Mar 17, 2025
  45. 2/3 git-compat-util: add NOT_CONSTANT macro and use it in atfork_prepare()Junio C Hamano, Mar 17, 2025
  46. Jeff KingMar 18, 2025
  47. Junio C HamanoMar 18, 2025
  48. Calvin WanMar 18, 2025
  49. Calvin WanMar 18, 2025
  50. Junio C HamanoMar 18, 2025
  51. 3/3 config.mak.dev: enable -Wunreachable-codeJunio C Hamano, Mar 17, 2025
  52. Jeff KingMar 18, 2025
  53. Karthik NayakMar 7, 2025
  54. Jeff KingMar 7, 2025
  55. Karthik NayakMar 7, 2025
  56. 7/8 refs: support partial update rejections during F/D checksKarthik Nayak, Mar 5, 2025
  57. 8/8 update-ref: add --allow-partial flag for stdin modeKarthik Nayak, Mar 5, 2025
  58. Junio C HamanoMar 5, 2025
  59. Karthik NayakMar 6, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.