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 <sahlberg@xxxxxxxxxx> --- 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html