Change ref_transaction_commit so that it does not free the transaction. Instead require that a caller will end a transaction by either calling ref_transaction_rollback or 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_rollback 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_rollback(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_rollback(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 | 8 ++++---- refs.c | 14 ++++++-------- refs.h | 14 +++++++++----- sequencer.c | 8 ++++---- 9 files changed, 28 insertions(+), 21 deletions(-) diff --git a/branch.c b/branch.c index 2aa5c82..8e58908 100644 --- a/branch.c +++ b/branch.c @@ -305,6 +305,7 @@ void create_branch(const char *head, ref_transaction_commit(transaction, msg, &err)) die_errno(_("%s: failed to write ref: %s"), ref.buf, err.buf); + ref_transaction_free(transaction); } if (real_ref && track) diff --git a/builtin/commit.c b/builtin/commit.c index a56f47a..b806c59 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1686,6 +1686,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) rollback_index_files(); die(_("HEAD: cannot update ref: %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 b037b29..5999f7d 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -165,6 +165,7 @@ static int replace_object(const char *object_ref, const char *replace_ref, ref_transaction_commit(transaction, NULL, &err)) die(_("%s: failed to replace ref: %s"), ref, err.buf); + ref_transaction_free(transaction); return 0; } diff --git a/builtin/tag.c b/builtin/tag.c index 31e32cb..4701bda 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -648,6 +648,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) 0, !is_null_sha1(prev)) || ref_transaction_commit(transaction, NULL, &err)) die(_("%s: cannot update the ref: %s"), ref.buf, 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 fc3512f..7fe9c11 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 35de36a..b3c6a04 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1703,16 +1703,16 @@ static int update_branch(struct branch *b) } } transaction = ref_transaction_begin(); - if ((!transaction || + if (!transaction || ref_transaction_update(transaction, b->name, b->sha1, old_sha1, - 0, 1)) || - (ref_transaction_commit(transaction, msg, &err) && - !(transaction = NULL))) { + 0, 1) || + ref_transaction_commit(transaction, msg, &err)) { ref_transaction_rollback(transaction); error("Unable to update branch %s: %s", b->name, err.buf); strbuf_release(&err); return -1; } + ref_transaction_free(transaction); return 0; } diff --git a/refs.c b/refs.c index c6ee43c..fcbdf9b 100644 --- a/refs.c +++ b/refs.c @@ -3302,7 +3302,7 @@ struct ref_transaction *ref_transaction_begin(void) return xcalloc(1, sizeof(struct ref_transaction)); } -static void ref_transaction_free(struct ref_transaction *transaction) +void ref_transaction_free(struct ref_transaction *transaction) { int i; @@ -3400,10 +3400,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)) || - (ref_transaction_commit(t, action, &err) && !(t = NULL))) { + !!oldval) || + ref_transaction_commit(t, action, &err)) { const char *str = "update_ref failed for ref '%s': %s"; ref_transaction_rollback(t); @@ -3417,6 +3417,7 @@ int update_ref(const char *action, const char *refname, strbuf_release(&err); return 1; } + ref_transaction_free(t); return 0; } @@ -3451,10 +3452,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); @@ -3520,7 +3519,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 5eb500e..fcde43e 100644 --- a/refs.h +++ b/refs.h @@ -210,8 +210,8 @@ enum action_on_err { /* * Begin a reference transaction. The reference transaction must - * eventually be commited using ref_transaction_commit() or rolled - * back using ref_transaction_rollback(). + * eventually be freed by either calling ref_transaction_rollback() + * or ref_transaction_free(). */ struct ref_transaction *ref_transaction_begin(void); @@ -267,13 +267,17 @@ 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. + * problem. If err is non-NULL we will add an error string to it to explain + * why the transaction failed. */ int ref_transaction_commit(struct ref_transaction *transaction, const char *msg, struct strbuf *err); +/* + * Free an existing transaction. + */ +void ref_transaction_free(struct ref_transaction *transaction); + /** Lock a ref and then write its file */ int update_ref(const char *action, const char *refname, const unsigned char *sha1, const unsigned char *oldval, diff --git a/sequencer.c b/sequencer.c index 9282a12..0cfdaf0 100644 --- a/sequencer.c +++ b/sequencer.c @@ -283,17 +283,17 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from, strbuf_addf(&sb, "%s: fast-forward", action_name(opts)); transaction = ref_transaction_begin(); - if ((!transaction || + if (!transaction || ref_transaction_update(transaction, "HEAD", to, from, - 0, !unborn)) || - (ref_transaction_commit(transaction, sb.buf, &err) && - !(transaction = NULL))) { + 0, !unborn) || + ref_transaction_commit(transaction, sb.buf, &err)) { ref_transaction_rollback(transaction); error(_("HEAD: Could not fast-forward: %s\n"), err.buf); strbuf_release(&sb); strbuf_release(&err); return -1; } + ref_transaction_free(transaction); strbuf_release(&sb); return 0; -- 1.9.1.532.gf8485a6 -- 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