Patrick Steinhardt <ps@xxxxxx> writes:
> static int s_update_ref(const char *action,
> struct ref *ref,
> + struct ref_transaction *transaction,
> int check_old)
> {
> char *msg;
> char *rla = getenv("GIT_REFLOG_ACTION");
> - struct ref_transaction *transaction;
> + struct ref_transaction *transaction_to_free = NULL;
> struct strbuf err = STRBUF_INIT;
> - int ret, df_conflict = 0;
> + int ret, df_conflict = 0, commit = 0;
> @@ -597,26 +598,38 @@ static int s_update_ref(const char *action,
> rla = default_rla.buf;
> msg = xstrfmt("%s: %s", rla, action);
>
> - transaction = ref_transaction_begin(&err);
> - if (!transaction ||
> - ref_transaction_update(transaction, ref->name,
> + /*
> + * If no transaction was passed to us, we manage the transaction
> + * ourselves. Otherwise, we trust the caller to handle the transaction
> + * lifecycle.
> + */
> + if (!transaction) {
> + transaction = transaction_to_free = ref_transaction_begin(&err);
> + if (!transaction)
> + goto fail;
> + commit = 1;
> + }
The idea is that we still allow the caller to pass NULL in which
case we begin and commit a transaction ourselves, but if the caller
is to pass an already initialized transaction to us, we obviously
do not have to "begin" but also we won't commit.
Which makes sense, but then do we still need the "commit" variable
that reminds us "we are responsible for finishing the transaction we
started"?
If we rename "transaction_to_free" to a name that makes it more
clear that it is a transaction that we created and that we are
responsible for seeing through to its end (after all, "to-free"
captures only one half of that, i.e. before returning, we are
responsible for calling ref_transation_free() on it), then we do not
need such an extra variable that can go out of sync by mistake, no?
The block that protects the call to ref_transaction_commit() can
just check if the transaction_to_free variable (with a better name)
is non-NULL, instead of looking at the commit variable.
Just my attempt to come up with an alternative name for
transaction_to_free:
- "our_transaction" because it is ours?
- "auto_transaction" because it is automatically started and
committed without bothering the caller?
