Currently, the trail subsystem of libsvn_fs_base does some pool-related stuff
that's not quite in line with Subversion's typically pool usage patterns. The
way it works is that higher-level callers in need of retry-able functionality
run svn_fs_base__retry() and svn_fs_base__retry_txn(), passing a "trail body"
callback function, a custom baton for that function's use, and a pool. The
retry() functions create a trail_t *trail, which has a ->pool member that is
defined to be a subpool of the pool passed to the retry function, and which is
promised *not* to be destroyed in the event that the retry functionality
succeeds. This allows the "trail body" functions (which accept only the baton
and the trail as parameters) to safely store returned data in trail->pool and
know that it will survive all this retry business.
The problem is that this burdens all those "trail body" functions with
maintaining still more scratch pools (as children of trail->pool, the only pool
they are sure to have access to), or forces higher-level consumers to provide
result pools in the batons (which adds complexity).
A better solution would be for the "trail body" functions to accept a
result_pool parameter, which is the same pool as was provided to the retry()
functions, and for the retry() functions to always destroy trail->pool before
exiting. This would allow the "trail body" functions to use result_pool for
data they need to return and trail->pool for scratch data. Score!
Improvement
Major