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;

        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

> --- a/refs.c
> +++ b/refs.c
> @@ -3303,6 +3303,9 @@ static void ref_transaction_free(struct ref_transaction 
> *transaction)
>  {
>       int i;
> +     if (!transaction)
> +             return;

Except for the unclear commit message,
Reviewed-by: Jonathan Nieder <>
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to