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.
> --- 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 <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
