Re: CVS update: subversion/subversion/libsvn_fs/tests skel-test.c

On Sun, Oct 08, 2000 at 12:36:52PM -0500, Jim Blandy wrote:
> Greg Stein <gstein@lyra.org> writes:
> > On Fri, Oct 06, 2000 at 06:04:40PM -0000, jimb@tigris.org wrote:
> > >...
> > > + svn_fs_kill_cleanup_node (c->pool, new);
> > > + svn_fs_close_node (new);
> >
> > Take a look at apr_run_cleanup(). It will run the cleanup and kill it in one
> > fell swoop.
>
> At the moment, I'm providing functions like this:
>
> /* Request a cleanup. */
> void svn_fs_cleanup_node (apr_pool_t *pool, svn_fs_node_t *node);
> /* Cancel a cleanup. */
> void svn_fs_kill_cleanup_node (apr_pool_t *pool, svn_fs_node_t *node);
>
> Basically, this hides the cleanup function itself from the caller.
> I'm not sure it's worth it.

Oh, I think that is quite fine, and feels right/worth it.

> Would you prefer that the fs simply export a function suitable for use
> as an APR cleanup, and then let user register, kill, or run it as they
> please?

Nope. I was merely suggesting that apr_run_cleanup() can be used in place
the pairs of functions that I saw in that checkin. So, you'd have a cover
function calls svn_fs_run_cleanup_node() or something.

Is that extra function worth it to save a few matched pairs? Dunno. I simply
saw the pair a few times, and suggested the run_cleanup. If it isn't a
common pattern, then don't worry about it -- the code is fine as it is.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/