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

[PATCH 01/20] refs.c: change ref_transaction_create to do error checking and return status

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Aug 27, 2014, 00:29 UTC
Message-ID
<20140827002904.GB20185@google.com>
In-Reply-To
<20140827002804.GA20185@google.com>
From: Ronnie Sahlberg <sahlberg@google.com>
Date: Wed, 16 Apr 2014 15:26:44 -0700

Do basic error checking in ref_transaction_create() and make it return non-zero on error. Update all callers to check the result of ref_transaction_create(). There are currently no conditions in _create that will return error but there will be in the future. Add an err argument that will be updated on failure.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/update-ref.c |  4 +++-
 refs.c               | 18 ++++++++++++------
 refs.h               | 48 +++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 3067b11..41121fa 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -226,7 +226,9 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)
 	if (*next != line_termination)
 		die("create %s: extra input: %s", refname, next);
 
-	ref_transaction_create(transaction, refname, new_sha1, update_flags);
+	if (ref_transaction_create(transaction, refname, new_sha1,
+				   update_flags, &err))
+		die("%s", err.buf);
 
 	update_flags = 0;
 	free(refname);
diff --git a/refs.c b/refs.c
index 3f05e88..c49f1c6 100644
--- a/refs.c
+++ b/refs.c
@@ -3449,18 +3449,24 @@ int ref_transaction_update(struct ref_transaction *transaction,
 	return 0;
 }
 
-void ref_transaction_create(struct ref_transaction *transaction,
-			    const char *refname,
-			    const unsigned char *new_sha1,
-			    int flags)
+int ref_transaction_create(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *new_sha1,
+			   int flags,
+			   struct strbuf *err)
 {
-	struct ref_update *update = add_update(transaction, refname);
+	struct ref_update *update;
+
+	if (!new_sha1 || is_null_sha1(new_sha1))
+		die("BUG: create ref with null new_sha1");
+
+	update = add_update(transaction, refname);
 
-	assert(!is_null_sha1(new_sha1));
 	hashcpy(update->new_sha1, new_sha1);
 	hashclr(update->old_sha1);
 	update->flags = flags;
 	update->have_old = 1;
+	return 0;
 }
 
 void ref_transaction_delete(struct ref_transaction *transaction,
diff --git a/refs.h b/refs.h
index c5376ce..b648819 100644
--- a/refs.h
+++ b/refs.h
@@ -10,6 +10,38 @@ struct ref_lock {
 	int force_write;
 };
 
+/*
+ * A ref_transaction represents a collection of ref updates
+ * that should succeed or fail together.
+ *
+ * Calling sequence
+ * ----------------
+ * - Allocate and initialize a `struct ref_transaction` by calling
+ *   `ref_transaction_begin()`.
+ *
+ * - List intended ref updates by calling functions like
+ *   `ref_transaction_update()` and `ref_transaction_create()`.
+ *
+ * - Call `ref_transaction_commit()` to execute the transaction.
+ *   If this succeeds, the ref updates will have taken place and
+ *   the transaction cannot be rolled back.
+ *
+ * - At any time call `ref_transaction_free()` to discard the
+ *   transaction and free associated resources.  In particular,
+ *   this rolls back the transaction if it has not been
+ *   successfully committed.
+ *
+ * Error handling
+ * --------------
+ *
+ * On error, transaction functions append a message about what
+ * went wrong to the 'err' argument.  The message mentions what
+ * ref was being updated (if any) when the error occurred so it
+ * can be passed to 'die' or 'error' as-is.
+ *
+ * The message is appended to err without first clearing err.
+ * err will not be '\n' terminated.
+ */
 struct ref_transaction;
 
 /*
@@ -248,7 +280,7 @@ struct ref_transaction *ref_transaction_begin(void);
  * it must not have existed beforehand.
  * Function returns 0 on success and non-zero on failure. A failure to update
  * means that the transaction as a whole has failed and will need to be
- * rolled back. On failure the err buffer will be updated.
+ * rolled back.
  */
 int ref_transaction_update(struct ref_transaction *transaction,
 			   const char *refname,
@@ -262,11 +294,15 @@ int ref_transaction_update(struct ref_transaction *transaction,
  * that the reference should have after the update; it must not be the
  * null SHA-1.  It is verified that the reference does not exist
  * already.
+ * Function returns 0 on success and non-zero on failure. A failure to create
+ * means that the transaction as a whole has failed and will need to be
+ * rolled back.
  */
-void ref_transaction_create(struct ref_transaction *transaction,
-			    const char *refname,
-			    const unsigned char *new_sha1,
-			    int flags);
+int ref_transaction_create(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *new_sha1,
+			   int flags,
+			   struct strbuf *err);
 
 /*
  * Add a reference deletion to transaction.  If have_old is true, then
@@ -282,8 +318,6 @@ void ref_transaction_delete(struct ref_transaction *transaction,
  * Commit all of the changes that have been queued in transaction, as
  * atomically as possible.  Return a nonzero value if there is a
  * problem.
- * If err is non-NULL we will add an error string to it to explain why
- * the transaction failed. The string does not end in newline.
  */
 int ref_transaction_commit(struct ref_transaction *transaction,
 			   const char *msg, struct strbuf *err);

-- 
2.1.0.rc2.206.gedb03e5
Previous: Jonathan NiederNext: Jonathan Nieder
Message 11 of 129 in “Transaction patch series overview”
  1. Ronnie SahlbergJul 30, 2014
  2. Ronnie SahlbergJul 31, 2014
  3. Ronnie SahlbergAug 8, 2014
  4. Ronnie SahlbergAug 19, 2014
  5. Junio C HamanoAug 19, 2014
  6. Jonathan NiederAug 20, 2014
  7. Jonathan NiederAug 26, 2014
  8. Junio C HamanoAug 26, 2014
  9. Jonathan NiederAug 26, 2014
  10. 0/20 rs/ref-transaction-1 (Re: Transaction patch series overview)Jonathan Nieder, Aug 27, 2014
  11. 01/20 refs.c: change ref_transaction_create to do error checking and return statusJonathan Nieder, Aug 27, 2014
  12. 02/20 refs.c: update ref_transaction_delete to check for error and return statusJonathan Nieder, Aug 27, 2014
  13. 03/20 refs.c: make ref_transaction_begin take an err argumentJonathan Nieder, Aug 27, 2014
  14. 04/20 refs.c: add transaction.status and track OPEN/CLOSEDJonathan Nieder, Aug 27, 2014
  15. 05/20 tag.c: use ref transactions when doing updatesJonathan Nieder, Aug 27, 2014
  16. 06/20 replace.c: use the ref transaction functions for updatesJonathan Nieder, Aug 27, 2014
  17. 07/20 commit.c: use ref transactions for updatesJonathan Nieder, Aug 27, 2014
  18. 08/20 sequencer.c: use ref transactions for all ref updatesJonathan Nieder, Aug 27, 2014
  19. 09/20 fast-import.c: change update_branch to use ref transactionsJonathan Nieder, Aug 27, 2014
  20. 10/20 branch.c: use ref transaction for all ref updatesJonathan Nieder, Aug 27, 2014
  21. 11/20 refs.c: change update_ref to use a transactionJonathan Nieder, Aug 27, 2014
  22. 12/20 receive-pack.c: use a reference transaction for updating the refsJonathan Nieder, Aug 27, 2014
  23. 13/20 fast-import.c: use a ref transaction when dumping tagsJonathan Nieder, Aug 27, 2014
  24. 14/20 walker.c: use ref transaction for ref updatesJonathan Nieder, Aug 27, 2014
  25. 15/20 refs.c: make lock_ref_sha1 staticJonathan Nieder, Aug 27, 2014
  26. 16/20 refs.c: remove the update_ref_lock functionJonathan Nieder, Aug 27, 2014
  27. 17/20 refs.c: remove the update_ref_write functionJonathan Nieder, Aug 27, 2014
  28. 18/20 refs.c: remove lock_ref_sha1Jonathan Nieder, Aug 27, 2014
  29. 19/20 refs.c: make prune_ref use a transaction to delete the refJonathan Nieder, Aug 27, 2014
  30. 20/20 refs.c: make delete_ref use a transactionJonathan Nieder, Aug 27, 2014
  31. Junio C HamanoAug 27, 2014
  32. Junio C HamanoAug 27, 2014
  33. 0/22 rs/ref-transaction-1 (Re: Transaction patch series overview)Jonathan Nieder, Sep 2, 2014
  34. 01/22 refs.c: change ref_transaction_create to do error checking and return statusJonathan Nieder, Sep 2, 2014
  35. 02/22 refs.c: update ref_transaction_delete to check for error and return statusJonathan Nieder, Sep 2, 2014
  36. 03/22 refs.c: make ref_transaction_begin take an err argumentJonathan Nieder, Sep 2, 2014
  37. 04/22 refs.c: add transaction.status and track OPEN/CLOSEDJonathan Nieder, Sep 2, 2014
  38. 05/22 tag.c: use ref transactions when doing updatesJonathan Nieder, Sep 2, 2014
  39. 06/22 replace.c: use the ref transaction functions for updatesJonathan Nieder, Sep 2, 2014
  40. 07/22 commit.c: use ref transactions for updatesJonathan Nieder, Sep 2, 2014
  41. 08/22 sequencer.c: use ref transactions for all ref updatesJonathan Nieder, Sep 2, 2014
  42. 09/22 fast-import.c: change update_branch to use ref transactionsJonathan Nieder, Sep 2, 2014
  43. 10/22 branch.c: use ref transaction for all ref updatesJonathan Nieder, Sep 2, 2014
  44. 11/22 refs.c: change update_ref to use a transactionJonathan Nieder, Sep 2, 2014
  45. 12/22 receive-pack.c: use a reference transaction for updating the refsJonathan Nieder, Sep 2, 2014
  46. 13/22 fast-import.c: use a ref transaction when dumping tagsJonathan Nieder, Sep 2, 2014
  47. 14/22 walker.c: use ref transaction for ref updatesJonathan Nieder, Sep 2, 2014
  48. 15/22 refs.c: make lock_ref_sha1 staticJonathan Nieder, Sep 2, 2014
  49. 16/22 refs.c: remove the update_ref_lock functionJonathan Nieder, Sep 2, 2014
  50. 17/22 refs.c: remove the update_ref_write functionJonathan Nieder, Sep 2, 2014
  51. 18/22 refs.c: remove lock_ref_sha1Jonathan Nieder, Sep 2, 2014
  52. 19/22 refs.c: make prune_ref use a transaction to delete the refJonathan Nieder, Sep 2, 2014
  53. 20/22 refs.c: make delete_ref use a transactionJonathan Nieder, Sep 2, 2014
  54. 21/22 update-ref --stdin: narrow scope of err strbufJonathan Nieder, Sep 2, 2014
  55. 22/22 update-ref --stdin: pass transaction around explicitlyJonathan Nieder, Sep 2, 2014
  56. Using Gerrit to review Git patches (was: Re: Transaction patch series overview)Michael Haggerty, Aug 27, 2014
  57. Shawn PearceAug 28, 2014
  58. 0/19 rs/ref-transaction (Re: Transaction patch series overview)Jonathan Nieder, Sep 11, 2014
  59. 01/19 mv test: recreate mod/ directory instead of relying on stale copyJonathan Nieder, Sep 11, 2014
  60. 02/19 wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as successJonathan Nieder, Sep 11, 2014
  61. 03/19 wrapper.c: add a new function unlink_or_msgJonathan Nieder, Sep 11, 2014
  62. 04/19 refs.c: add an err argument to delete_ref_looseJonathan Nieder, Sep 11, 2014
  63. 05/19 refs.c: pass the ref log message to _create/delete/update instead of _commitJonathan Nieder, Sep 11, 2014
  64. 06/19 rename_ref: don't ask read_ref_full where the ref came fromJonathan Nieder, Sep 11, 2014
  65. 07/19 refs.c: move the check for valid refname to lock_ref_sha1_basicJonathan Nieder, Sep 11, 2014
  66. 08/19 refs.c: call lock_ref_sha1_basic directly from commitJonathan Nieder, Sep 11, 2014
  67. 09/19 refs.c: pass a skip list to name_conflict_fnJonathan Nieder, Sep 11, 2014
  68. 10/19 refs.c: ref_transaction_commit: distinguish name conflicts from other errorsJonathan Nieder, Sep 11, 2014
  69. 11/19 fetch.c: change s_update_ref to use a ref transactionJonathan Nieder, Sep 11, 2014
  70. 12/19 refs.c: make write_ref_sha1 staticJonathan Nieder, Sep 11, 2014
  71. 13/19 refs.c: change resolve_ref_unsafe reading argument to be a flags fieldJonathan Nieder, Sep 11, 2014
  72. 14/19 branch -d: avoid repeated symref resolutionJonathan Nieder, Sep 11, 2014
  73. 15/19 refs.c: fix handling of badly named refsJonathan Nieder, Sep 11, 2014
  74. 16/19 for-each-ref.c: improve message before aborting on broken refJonathan Nieder, Sep 11, 2014
  75. 17/19 refs.c: do not permit err == NULLJonathan Nieder, Sep 11, 2014
  76. 18/19 lockfile: remove unable_to_lock_errorJonathan Nieder, Sep 11, 2014
  77. 19/19 ref_transaction_commit: bail out on failure to remove a refJonathan Nieder, Sep 11, 2014
  78. Junio C HamanoSep 11, 2014
  79. Junio C HamanoSep 11, 2014
  80. Jonathan NiederSep 12, 2014
  81. Junio C HamanoSep 12, 2014
  82. Jonathan NiederSep 12, 2014
  83. Junio C HamanoSep 12, 2014
  84. Junio C HamanoSep 12, 2014
  85. Junio C HamanoSep 13, 2014
  86. Michael HaggertySep 12, 2014
  87. Jonathan NiederSep 12, 2014
  88. Michael HaggertySep 17, 2014
  89. Junio C HamanoSep 18, 2014
  90. Jonathan NiederSep 18, 2014
  91. Junio C HamanoSep 18, 2014
  92. Jonathan NiederSep 18, 2014
  93. Junio C HamanoSep 25, 2014
  94. Jonathan NiederSep 25, 2014
  95. Junio C HamanoSep 25, 2014
  96. 0/24 rs/ref-transactionJonathan Nieder, Oct 2, 2014
  97. 01/24 mv test: recreate mod/ directory instead of relying on stale copyJonathan Nieder, Oct 2, 2014
  98. 02/24 wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as successJonathan Nieder, Oct 2, 2014
  99. 03/24 wrapper.c: add a new function unlink_or_msgJonathan Nieder, Oct 2, 2014
  100. 04/24 refs.c: add an err argument to delete_ref_looseJonathan Nieder, Oct 2, 2014
  101. 05/24 refs.c: pass the ref log message to _create/delete/update instead of _commitJonathan Nieder, Oct 2, 2014
  102. 06/24 rename_ref: don't ask read_ref_full where the ref came fromJonathan Nieder, Oct 2, 2014
  103. 07/24 refs.c: refuse to lock badly named refs in lock_ref_sha1_basicJonathan Nieder, Oct 2, 2014
  104. 08/24 refs.c: call lock_ref_sha1_basic directly from commitJonathan Nieder, Oct 2, 2014
  105. 09/24 refs.c: pass a list of names to skip to is_refname_availableJonathan Nieder, Oct 2, 2014
  106. Junio C HamanoOct 2, 2014
  107. Jonathan NiederOct 3, 2014
  108. Junio C HamanoOct 3, 2014
  109. 09/24 refs.c: pass a list of names to skip to is_refname_availableJonathan Nieder, Oct 3, 2014
  110. Junio C HamanoOct 7, 2014
  111. 10/24 refs.c: ref_transaction_commit: distinguish name conflicts from other errorsJonathan Nieder, Oct 2, 2014
  112. 11/24 fetch.c: change s_update_ref to use a ref transactionJonathan Nieder, Oct 2, 2014
  113. 12/24 refs.c: make write_ref_sha1 staticJonathan Nieder, Oct 2, 2014
  114. 13/24 refs.c: change resolve_ref_unsafe reading argument to be a flags fieldJonathan Nieder, Oct 2, 2014
  115. 14/24 reflog test: test interaction with detached HEADJonathan Nieder, Oct 2, 2014
  116. 15/24 branch -d: avoid repeated symref resolutionJonathan Nieder, Oct 2, 2014
  117. 16/24 branch -d: simplify by using RESOLVE_REF_READING flagJonathan Nieder, Oct 2, 2014
  118. 17/24 packed-ref cache: forbid dot-components in refnamesJonathan Nieder, Oct 2, 2014
  119. 18/24 test: put tests for handling of bad ref names in one placeJonathan Nieder, Oct 2, 2014
  120. 19/24 refs.c: allow listing and deleting badly named refsJonathan Nieder, Oct 2, 2014
  121. Junio C HamanoOct 2, 2014
  122. Ronnie SahlbergOct 3, 2014
  123. Ronnie SahlbergOct 3, 2014
  124. Junio C HamanoOct 3, 2014
  125. 20/24 for-each-ref.c: improve message before aborting on broken refJonathan Nieder, Oct 2, 2014
  126. 21/24 remote rm/prune: print a message when writing packed-refs failsJonathan Nieder, Oct 2, 2014
  127. 22/24 refs.c: do not permit err == NULLJonathan Nieder, Oct 2, 2014
  128. 23/24 lockfile: remove unable_to_lock_errorJonathan Nieder, Oct 2, 2014
  129. 24/24 ref_transaction_commit: bail out on failure to remove a refJonathan Nieder, Oct 2, 2014

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.