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

Re: [PATCH v2 2/4] fetch: refactor `s_update_ref` to use common exit path

From
Patrick Steinhardt <ps@pks.im>
Date
Jan 11, 2021, 10:28 UTC
Message-ID
<X/woXrxFqctcX2NB@ncase>
In-Reply-To
<xmqqim87gf9z.fsf@gitster.c.googlers.com>
On Fri, Jan 08, 2021 at 03:50:00PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
Show 30 quoted lines
> >  	ret = ref_transaction_commit(transaction, &err);
> >  	if (ret) {
> > -		df_conflict = (ret == TRANSACTION_NAME_CONFLICT);
> > -		goto fail;
> > +		ret = (ret == TRANSACTION_NAME_CONFLICT) ? STORE_REF_ERROR_DF_CONFLICT
> > +							 : STORE_REF_ERROR_OTHER;
> > +		goto out;
> >  	}
> > 
> > +out:
> >  	ref_transaction_free(transaction);
> 
> It is a bit funny to see a goto that jumps to the label without
> having anything else in between, but we know we will be adding more
> code just before the "out:" label, so it is a good preliminary
> preparation.
> 
> I think a variant that is much easier to follow would be to write
> like this instead:
> 
> 	switch (ref_transaction_commit(transaction, &err)) {
>         case 0: /* happy */
> 		break;
> 	case TRANSACTION_NAME_CONFLICT:
> 		ret = STORE_REF_ERROR_DF_CONFLICT;
> 		goto out;
> 	default:
> 		ret = STORE_REF_ERROR_OTHER;
> 		goto out;
> 	}
Agreed, that is easier to read. Thanks!
Patrick
Previous: Junio C HamanoNext: Patrick Steinhardt
Message 17 of 41 in “fetch: implement support for atomic reference updates”
  1. 0/2 fetch: implement support for atomic reference updatesPatrick Steinhardt, Jan 7, 2021
  2. 1/2 fetch: allow passing a transaction to `s_update_ref()`Patrick Steinhardt, Jan 7, 2021
  3. Junio C HamanoJan 7, 2021
  4. Christian CouderJan 8, 2021
  5. Junio C HamanoJan 8, 2021
  6. 2/2 fetch: implement support for atomic reference updatesPatrick Steinhardt, Jan 7, 2021
  7. Junio C HamanoJan 8, 2021
  8. 0/4 fetch: implement support for atomic reference updatesPatrick Steinhardt, Jan 8, 2021
  9. 1/4 fetch: extract writing to FETCH_HEADPatrick Steinhardt, Jan 8, 2021
  10. Junio C HamanoJan 8, 2021
  11. Patrick SteinhardtJan 11, 2021
  12. Junio C HamanoJan 11, 2021
  13. 3/4 fetch: allow passing a transaction to `s_update_ref()`Patrick Steinhardt, Jan 8, 2021
  14. Junio C HamanoJan 8, 2021
  15. 2/4 fetch: refactor `s_update_ref` to use common exit pathPatrick Steinhardt, Jan 8, 2021
  16. Junio C HamanoJan 8, 2021
  17. Patrick SteinhardtJan 11, 2021
  18. 4/4 fetch: implement support for atomic reference updatesPatrick Steinhardt, Jan 8, 2021
  19. Junio C HamanoJan 9, 2021
  20. Patrick SteinhardtJan 11, 2021
  21. Junio C HamanoJan 11, 2021
  22. Patrick SteinhardtJan 12, 2021
  23. Patrick SteinhardtJan 12, 2021
  24. Junio C HamanoJan 12, 2021
  25. 0/5 fetch: implement support for atomic reference updatesPatrick Steinhardt, Jan 11, 2021
  26. 1/5 fetch: extract writing to FETCH_HEADPatrick Steinhardt, Jan 11, 2021
  27. Junio C HamanoJan 11, 2021
  28. 2/5 fetch: use strbuf to format FETCH_HEAD updatesPatrick Steinhardt, Jan 11, 2021
  29. Patrick SteinhardtJan 11, 2021
  30. Christian CouderJan 11, 2021
  31. Junio C HamanoJan 11, 2021
  32. 4/5 fetch: allow passing a transaction to `s_update_ref()`Patrick Steinhardt, Jan 11, 2021
  33. 3/5 fetch: refactor `s_update_ref` to use common exit pathPatrick Steinhardt, Jan 11, 2021
  34. 5/5 fetch: implement support for atomic reference updatesPatrick Steinhardt, Jan 11, 2021
  35. Junio C HamanoJan 12, 2021
  36. 0/5 fetch: implement support for atomic reference updatesPatrick Steinhardt, Jan 12, 2021
  37. 3/5 fetch: refactor `s_update_ref` to use common exit pathPatrick Steinhardt, Jan 12, 2021
  38. 1/5 fetch: extract writing to FETCH_HEADPatrick Steinhardt, Jan 12, 2021
  39. 2/5 fetch: use strbuf to format FETCH_HEAD updatesPatrick Steinhardt, Jan 12, 2021
  40. 4/5 fetch: allow passing a transaction to `s_update_ref()`Patrick Steinhardt, Jan 12, 2021
  41. 5/5 fetch: implement support for atomic reference updatesPatrick Steinhardt, Jan 12, 2021

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.