Re: [PATCH v20 23/48] refs.c: add transaction.status and track OPEN/CLOSED/ERROR

I updated the comments.

Status is used in a later series to track certain errno settings. This
used to be done here but was moved to a later series.
I removed the status field for now and will re add it later when we
start using it.

Thanks!


On Tue, Jul 8, 2014 at 5:00 AM, Michael Haggerty [off-list ref] wrote:

On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:

quoted

Track the status of a transaction in a new status field. Check the field for

The status field is not set or used anywhere.  The field that you use is
"state".

quoted

sanity, i.e. that status must be OPEN when _commit/_create/_delete or
_update is called or else die(BUG:...)

Signed-off-by: Ronnie Sahlberg <redacted>
---
 refs.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 9cb7908..8c695ba 100644
--- a/refs.c
+++ b/refs.c

@@ -3387,6 +3387,25 @@ struct ref_update {
 };

 /*
+ * Transaction states.
+ * OPEN:   The transaction is in a valid state and can accept new updates.
+ *         An OPEN transaction can be committed.
+ * CLOSED: If an open transaction is successfully committed the state will
+ *         change to CLOSED. No further changes can be made to a CLOSED
+ *         transaction.
+ *         CLOSED means that all updates have been successfully committed and
+ *         the only thing that remains is to free the completed transaction.
+ * ERROR:  The transaction has failed and is no longer committable.
+ *         No further changes can be made to a CLOSED transaction and it must
+ *         be rolled back using transaction_free.
+ */
+enum ref_transaction_state {
+     REF_TRANSACTION_OPEN   = 0,
+     REF_TRANSACTION_CLOSED = 1,
+     REF_TRANSACTION_ERROR  = 2,
+};
+
+/*
  * Data structure for holding a reference transaction, which can
  * consist of checks and updates to multiple references, carried out
  * as atomically as possible.  This structure is opaque to callers.

@@ -3395,6 +3414,8 @@ struct ref_transaction {
      struct ref_update **updates;
      size_t alloc;
      size_t nr;
+     enum ref_transaction_state state;
+     int status;

The status field should probably be deleted.

quoted

 };

 struct ref_transaction *ref_transaction_begin(struct strbuf *err)

@@ -3437,6 +3458,9 @@ int ref_transaction_update(struct ref_transaction *transaction,
 {
      struct ref_update *update;

+     if (transaction->state != REF_TRANSACTION_OPEN)
+             die("BUG: update called for transaction that is not open");
+
      if (have_old && !old_sha1)
              die("BUG: have_old is true but old_sha1 is NULL");

@@ -3457,6 +3481,9 @@ int ref_transaction_create(struct ref_transaction *transaction,
 {
      struct ref_update *update;

+     if (transaction->state != REF_TRANSACTION_OPEN)
+             die("BUG: create called for transaction that is not open");
+
      if (!new_sha1 || is_null_sha1(new_sha1))
              die("BUG: create ref with null new_sha1");

@@ -3477,6 +3504,9 @@ int ref_transaction_delete(struct ref_transaction *transaction,
 {
      struct ref_update *update;

+     if (transaction->state != REF_TRANSACTION_OPEN)
+             die("BUG: delete called for transaction that is not open");
+
      if (have_old && !old_sha1)
              die("BUG: have_old is true but old_sha1 is NULL");

@@ -3532,8 +3562,13 @@ int ref_transaction_commit(struct ref_transaction *transaction,
      int n = transaction->nr;
      struct ref_update **updates = transaction->updates;

-     if (!n)
+     if (transaction->state != REF_TRANSACTION_OPEN)
+             die("BUG: commit called for transaction that is not open");
+
+     if (!n) {
+             transaction->state = REF_TRANSACTION_CLOSED;
              return 0;
+     }

      /* Allocate work space */
      delnames = xmalloc(sizeof(*delnames) * n);

@@ -3595,6 +3630,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,
      clear_loose_ref_cache(&ref_cache);

 cleanup:
+     transaction->state = ret ? REF_TRANSACTION_ERROR
+             : REF_TRANSACTION_CLOSED;
+
      for (i = 0; i < n; i++)
              if (updates[i]->lock)
                      unlock_ref(updates[i]->lock);

--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/