Thanks. I used your improved text in the commit message. On Thu, May 15, 2014 at 11:15 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > Ronnie Sahlberg wrote: > >> Allow ref_transaction_free to be called with NULL and as a result allow >> ref_transaction_rollback to be called for a NULL transaction. >> >> This allows us to write code that will >> if ( (!transaction || >> ref_transaction_update(...)) || >> (ref_transaction_commit(...) && !(transaction = NULL)) { >> ref_transaction_rollback(transaction); >> ... >> } >> >> In this case transaction is reset to NULL IFF ref_transaction_commit() was >> invoked and thus the rollback becomes ref_transaction_rollback(NULL) which >> is safe. IF the conditional triggered prior to ref_transaction_commit() >> then transaction is untouched and then ref_transaction_rollback(transaction) >> will rollback the failed transaction. > > I still think these last two paragraphs confuse more than enlighten > here. There's plenty of time to explain them in the patch that uses > that code. > > I'd just say something like > > Allow ref_transaction_free(NULL) and hence ref_transaction_rollback(NULL) > as no-ops. > > This makes ref_transaction_rollback easier to use and more similar to > plain 'free'. > > And maybe: > > In particular, it lets us rollback unconditionally as part of cleanup > code after setting 'transaction = NULL' if a transaction has been > committed or rolled back already. > > Thanks, > Jonathan -- 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