Change ref_transaction_commit so that it does not free the transaction.
Instead require that a caller will end a transaction by calling
ref_transaction_free.

By having the transaction object remaining valid after _commit returns allows
us to write much nicer code and still be able to call ref_transaction_free
safely. Instead of this horribleness
        t = ref_transaction_begin();
        if ((!t ||
            ref_transaction_update(t, refname, sha1, oldval, flags,
                                   !!oldval)) ||
            (ref_transaction_commit(t, action, &err) && !(t = NULL))) {
                ref_transaction_free(t);

we can now just do the much nicer
        t = ref_transaction_begin();
        if (!t ||
            ref_transaction_update(t, refname, sha1, oldval, flags,
                                   !!oldval) ||
            ref_transaction_commit(&t, action, &err)) {
                ref_transaction_free(t);
                ... die/return ...

        ref_transaction_free(transaction);

Signed-off-by: Ronnie Sahlberg <sahlb...@google.com>
---
 branch.c             |  1 +
 builtin/commit.c     |  1 +
 builtin/replace.c    |  1 +
 builtin/tag.c        |  1 +
 builtin/update-ref.c |  1 +
 fast-import.c        |  4 ++--
 refs.c               | 12 +++++-------
 refs.h               |  8 +++-----
 sequencer.c          |  4 ++--
 9 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/branch.c b/branch.c
index 8bf7588..f78a28b 100644
--- a/branch.c
+++ b/branch.c
@@ -304,6 +304,7 @@ void create_branch(const char *head,
                                           null_sha1, 0, !forcing, &err) ||
                    ref_transaction_commit(transaction, msg, &err))
                        die("%s", err.buf);
+               ref_transaction_free(transaction);
        }
 
        if (real_ref && track)
diff --git a/builtin/commit.c b/builtin/commit.c
index c429216..6b888f2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1726,6 +1726,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
                rollback_index_files();
                die("%s", err.buf);
        }
+       ref_transaction_free(transaction);
 
        unlink(git_path("CHERRY_PICK_HEAD"));
        unlink(git_path("REVERT_HEAD"));
diff --git a/builtin/replace.c b/builtin/replace.c
index e8932cd..af7f72d 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -164,6 +164,7 @@ static int replace_object_sha1(const char *object_ref,
            ref_transaction_commit(transaction, NULL, &err))
                die("%s", err.buf);
 
+       ref_transaction_free(transaction);
        return 0;
 }
 
diff --git a/builtin/tag.c b/builtin/tag.c
index b05f9a5..30b471c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -708,6 +708,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
                                   0, !is_null_sha1(prev), &err) ||
            ref_transaction_commit(transaction, NULL, &err))
                die("%s", err.buf);
+       ref_transaction_free(transaction);
        if (force && !is_null_sha1(prev) && hashcmp(prev, object))
                printf(_("Updated tag '%s' (was %s)\n"), tag, 
find_unique_abbrev(prev, DEFAULT_ABBREV));
 
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index cdb71a8..4c2145e 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -373,6 +373,7 @@ int cmd_update_ref(int argc, const char **argv, const char 
*prefix)
                update_refs_stdin();
                if (ref_transaction_commit(transaction, msg, &err))
                        die("%s", err.buf);
+               ref_transaction_free(transaction);
                return 0;
        }
 
diff --git a/fast-import.c b/fast-import.c
index 60d4538..cb3f5af 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1710,13 +1710,13 @@ static int update_branch(struct branch *b)
        if (!transaction ||
            ref_transaction_update(transaction, b->name, b->sha1, old_sha1,
                                   0, 1, &err) ||
-           (ref_transaction_commit(transaction, msg, &err) &&
-            !(transaction = NULL))) {
+           ref_transaction_commit(transaction, msg, &err)) {
                ref_transaction_free(transaction);
                error("%s", err.buf);
                strbuf_release(&err);
                return -1;
        }
+       ref_transaction_free(transaction);
        return 0;
 }
 
diff --git a/refs.c b/refs.c
index 2c3f070..266a792 100644
--- a/refs.c
+++ b/refs.c
@@ -3446,10 +3446,10 @@ int update_ref(const char *action, const char *refname,
        struct strbuf err = STRBUF_INIT;
 
        t = ref_transaction_begin();
-       if ((!t ||
+       if (!t ||
            ref_transaction_update(t, refname, sha1, oldval, flags,
-                                  !!oldval, &err)) ||
-           (ref_transaction_commit(t, action, &err) && !(t = NULL))) {
+                                  !!oldval, &err) ||
+           ref_transaction_commit(t, action, &err)) {
                const char *str = "update_ref failed for ref '%s': %s";
 
                ref_transaction_free(t);
@@ -3463,6 +3463,7 @@ int update_ref(const char *action, const char *refname,
                strbuf_release(&err);
                return 1;
        }
+       ref_transaction_free(t);
        return 0;
 }
 
@@ -3497,10 +3498,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
        int n = transaction->nr;
        struct ref_update **updates = transaction->updates;
 
-       if (!n) {
-               ref_transaction_free(transaction);
+       if (!n)
                return 0;
-       }
 
        /* Allocate work space */
        delnames = xmalloc(sizeof(*delnames) * n);
@@ -3567,7 +3566,6 @@ cleanup:
                if (updates[i]->lock)
                        unlock_ref(updates[i]->lock);
        free(delnames);
-       ref_transaction_free(transaction);
        return ret;
 }
 
diff --git a/refs.h b/refs.h
index 9b978da..e0f5f67 100644
--- a/refs.h
+++ b/refs.h
@@ -216,8 +216,7 @@ enum action_on_err {
 
 /*
  * Begin a reference transaction.  The reference transaction must
- * eventually be commited using ref_transaction_commit() or freed by
- * calling ref_transaction_free().
+ * eventually be freed by calling ref_transaction_free().
  */
 struct ref_transaction *ref_transaction_begin(void);
 
@@ -278,9 +277,8 @@ int 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.  The ref_transaction is freed by this function.
- * 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.
+ * 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);
diff --git a/sequencer.c b/sequencer.c
index b047fb8..a2bc9e1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -286,14 +286,14 @@ static int fast_forward_to(const unsigned char *to, const 
unsigned char *from,
        if (!transaction ||
            ref_transaction_update(transaction, "HEAD", to, from,
                                   0, !unborn, &err) ||
-           (ref_transaction_commit(transaction, sb.buf, &err) &&
-            !(transaction = NULL))) {
+           ref_transaction_commit(transaction, sb.buf, &err)) {
                ref_transaction_free(transaction);
                error("%s", err.buf);
                strbuf_release(&sb);
                strbuf_release(&err);
                return -1;
        }
+       ref_transaction_free(transaction);
 
        strbuf_release(&sb);
        return 0;
-- 
2.0.0.rc3.510.g20c254b

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to