[RFC] New macro SVN_WC__DB_WITH_TXN(my_func_call(...)) doesn't need a baton
From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 17 Dec 2012 23:32:09 +0000 (GMT)
The "call a function within a transaction" paradigm is used heavily in wc_db.c. The current implementation uses a callback function with a standard signature, passing all other parameters to it via a "baton":
struct base_remove_baton {
param1;
param2;
...
};
/* ...
Implements svn_wc__db_txn_callback_t. */
static svn_error_t *db_base_remove(void *baton,
wcroot,
local_relpath,
scratch_pool)
{
struct base_remove_baton *rb = baton;
... rb->param1 ...
}
In the caller:
{
struct base_remove_baton rb;
rb.param1 = param1;
rb.param2 = param2;
...
SVN_ERR(svn_wc__db_with_txn(wcroot, local_relpath, db_base_remove,
&rb, scratch_pool));
}
But this is unnecessarily verbose. To simplify it, we can write a statement macro that takes a complete, arbitrary function call as an argument. Pseudo-code (omitting error handling):
#define WITH_TXN(db, wrapped_function_call)
{
start_txn(db)
if (wrapped_function_call) succeeds:
commit_txn(db)
else:
roll_back_txn(db)
}
(The argument need not be a function call, it can be any expression of type svn_error_t*.)
Then the usage looks like this:
/* ... */
static svn_error_t *db_base_remove(param1,
param2,
...,
scratch_pool)
{
... param1 ...
}
In the caller:
{
SVN_WC__DB_WITH_TXN(wcroot,
db_base_remove(param1, param2, ..., scratch_pool));
}
This style can be used not only for DB transactions but anywhere we need to wrap a call inside a resource acquisition and release. We already use this style for working with mutex locks:
SVN_MUTEX__WITH_LOCK(mutex, expr)
And this style could also be used in place of:
with_txnlist_lock() in fs_fs.c
and probably others.
The main advantage is:
* Simpler usage.
Potential down-sides:
* Macros can be harder to debug in some environments? But we already
accept that svn_wc__db_with_txn is wrapped in SVN_ERR so I don't see a
problem.
The attached patch contains an implementation and some examples of usage. I would tweak the macro definition and support functions a bit, and there is a bug in that patch somewhere, but you get the idea.
I would prefer to be working with the simpler style of code, so I would like to introduce and use this macro.
What does anybody think of introducing this form, in principle and in practice?
- Julian
--
Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
|
This is an archived mail posted to the Subversion Dev mailing list.
This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.