Re: [PATCH v6 02/42] refs.c: allow passing NULL to ref_transaction_free

Ronnie Sahlberg wrote:

Allow ref_transaction_free to be called with NULL and in extension allow
ref_transaction_rollback to be called for a NULL transaction.

In extension = as a result?

Makes sense.  It lets someone do the usual

	struct ref_transaction *transaction;
	int ret = 0;

	if (something_fails()) {
		ret = -1;
		goto cleanup;
	}
	...

 cleanup:
	ref_transaction_free(transaction);
	return ret;

just like you can already do with free().

This allows us to write code that will

  if ( (!transaction ||
        ref_transaction_update(...))  ||
      (ref_transaction_commit(...) && !(transaction = NULL)) {
          ref_transaction_rollback(transaction);
          ...
  }

Somewhere in the whitespace and parentheses I'm lost.

Is the idea that when ref_transaction_commit fails it will have
freed the transaction so we need not to roll back to prevent a
double free?  I think it would be simpler for the caller to
unconditionally set transaction to NULL after calling
ref_transaction_commit in such a case to avoid use-after-free.

Even better if it is the caller's responsibility to free
the transaction.  At any rate, it doesn't seem related to this
patch.

Except for the unclear commit message,
Reviewed-by: Jonathan Nieder <redacted>