You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/04/16 14:48:08 UTC

Re: svn commit: r934599 - in /subversion/trunk/subversion: include/private/ libsvn_fs_base/bdb/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_subr/ svnserve/

On Thu, Apr 15, 2010 at 17:38,  <da...@apache.org> wrote:
> Author: danielsh
> Date: Thu Apr 15 21:38:14 2010
> New Revision: 934599
>
> URL: http://svn.apache.org/viewvc?rev=934599&view=rev
> Log:
> Update the svn_atomic__init_once() interface to take a baton, and update all
> callers.  The new caller (that needs the baton) will be added shortly.
>
> * subversion/include/private/svn_atomic.h,
>  subversion/libsvn_subr/atomic.c:
>  (svn_atomic__init_once):
>    Add a BATON parameter to be passed to INIT_FUNC, and update INIT_FUNC's
>    signature to accept it.
>
> * subversion/libsvn_fs_base/bdb/env.c
>  subversion/libsvn_ra_neon/session.c,
>  subversion/libsvn_ra_serf/win32_auth_sspi.c,
>  subversion/libsvn_ra_svn/cyrus_auth.c,
>  subversion/libsvn_subr/io.c,
>  subversion/libsvn_subr/sqlite.c,
>  subversion/libsvn_subr/win32_xlate.c,
>  subversion/svnserve/cyrus_auth.c:
>    Update svn_atomic__init_once() calls and callbacks to pass/accept a baton.

The pool parameter seems redundant now. Should a function require a
pool, it can just pass it as the baton, or within the baton.

Cheers,
-g

Re: svn commit: r934599 - in /subversion/trunk/subversion: include/private/ libsvn_fs_base/bdb/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_subr/ svnserve/

Posted by Julian Foad <ju...@wandisco.com>.
Daniel Shahaf wrote:
> Greg Stein wrote on Fri, 16 Apr 2010 at 10:48 -0400:
> > On Thu, Apr 15, 2010 at 17:38,  <da...@apache.org> wrote:
> > > Author: danielsh
> > > Date: Thu Apr 15 21:38:14 2010
> > > New Revision: 934599
> > >
> > > URL: http://svn.apache.org/viewvc?rev=934599&view=rev
> > > Log:
> > > Update the svn_atomic__init_once() interface to take a baton, and update all
> > > callers.  The new caller (that needs the baton) will be added shortly.
> > >
> > > * subversion/include/private/svn_atomic.h,
> > >  subversion/libsvn_subr/atomic.c:
> > >  (svn_atomic__init_once):
> > >    Add a BATON parameter to be passed to INIT_FUNC, and update INIT_FUNC's
> > >    signature to accept it.
> > >
> > > * subversion/libsvn_fs_base/bdb/env.c
> > >  subversion/libsvn_ra_neon/session.c,
> > >  subversion/libsvn_ra_serf/win32_auth_sspi.c,
> > >  subversion/libsvn_ra_svn/cyrus_auth.c,
> > >  subversion/libsvn_subr/io.c,
> > >  subversion/libsvn_subr/sqlite.c,
> > >  subversion/libsvn_subr/win32_xlate.c,
> > >  subversion/svnserve/cyrus_auth.c:
> > >    Update svn_atomic__init_once() calls and callbacks to pass/accept a baton.
> > 
> > The pool parameter seems redundant now. Should a function require a
> > pool, it can just pass it as the baton, or within the baton.
> > 
> 
> We have plenty of callback types [e.g., 1,2] that accept both a baton
> and a pool.  And probably a few that go the other way (e.g., [3,4]).
> Do we have a preference these days, one way or the other?

Not a direct answer to this case, but ...

A baton can contain a long-lived pool, useful as a "result pool".  For
callbacks which may be called an arbitrary number of times from a single
caller, we should provide a scratch pool with a lifetime of just during
one call.  This can't be inside the baton because the caller (that's
calling the callback function) doesn't know how to see into the baton.

However, the atomic init callback isn't one that's called an arbitrary
number of times so that argument doesn't apply.

- Julian


> (FWIW, [1] accepts an baton+pool in trunk, but only a baton (with pool,
> presumably, in the baton) in 1.6.)
> 
> Daniel
> 
> 
> 
> [1] svn_sqlite__transaction_callback_t
> [2] svn_ra_progress_notify_func_t
> [3] svn_cancel_func_t
> [4] svn_close_fn_t



Re: svn commit: r934599 - in /subversion/trunk/subversion: include/private/ libsvn_fs_base/bdb/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_subr/ svnserve/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Fri, 16 Apr 2010 at 10:48 -0400:
> On Thu, Apr 15, 2010 at 17:38,  <da...@apache.org> wrote:
> > Author: danielsh
> > Date: Thu Apr 15 21:38:14 2010
> > New Revision: 934599
> >
> > URL: http://svn.apache.org/viewvc?rev=934599&view=rev
> > Log:
> > Update the svn_atomic__init_once() interface to take a baton, and update all
> > callers.  The new caller (that needs the baton) will be added shortly.
> >
> > * subversion/include/private/svn_atomic.h,
> >  subversion/libsvn_subr/atomic.c:
> >  (svn_atomic__init_once):
> >    Add a BATON parameter to be passed to INIT_FUNC, and update INIT_FUNC's
> >    signature to accept it.
> >
> > * subversion/libsvn_fs_base/bdb/env.c
> >  subversion/libsvn_ra_neon/session.c,
> >  subversion/libsvn_ra_serf/win32_auth_sspi.c,
> >  subversion/libsvn_ra_svn/cyrus_auth.c,
> >  subversion/libsvn_subr/io.c,
> >  subversion/libsvn_subr/sqlite.c,
> >  subversion/libsvn_subr/win32_xlate.c,
> >  subversion/svnserve/cyrus_auth.c:
> >    Update svn_atomic__init_once() calls and callbacks to pass/accept a baton.
> 
> The pool parameter seems redundant now. Should a function require a
> pool, it can just pass it as the baton, or within the baton.
> 

We have plenty of callback types [e.g., 1,2] that accept both a baton
and a pool.  And probably a few that go the other way (e.g., [3,4]).
Do we have a preference these days, one way or the other?

(FWIW, [1] accepts an baton+pool in trunk, but only a baton (with pool,
presumably, in the baton) in 1.6.)

Daniel



[1] svn_sqlite__transaction_callback_t
[2] svn_ra_progress_notify_func_t
[3] svn_cancel_func_t
[4] svn_close_fn_t


> Cheers,
> -g
> 

Re: svn commit: r934599 - in /subversion/trunk/subversion: include/private/ libsvn_fs_base/bdb/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_subr/ svnserve/

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Fri, Apr 16, 2010 at 9:48 AM, Greg Stein <gs...@gmail.com> wrote:

> On Thu, Apr 15, 2010 at 17:38,  <da...@apache.org> wrote:
> > Author: danielsh
> > Date: Thu Apr 15 21:38:14 2010
> > New Revision: 934599
> >
> > URL: http://svn.apache.org/viewvc?rev=934599&view=rev
> > Log:
> > Update the svn_atomic__init_once() interface to take a baton, and update
> all
> > callers.  The new caller (that needs the baton) will be added shortly.
> >
> > * subversion/include/private/svn_atomic.h,
> >  subversion/libsvn_subr/atomic.c:
> >  (svn_atomic__init_once):
> >    Add a BATON parameter to be passed to INIT_FUNC, and update
> INIT_FUNC's
> >    signature to accept it.
> >
> > * subversion/libsvn_fs_base/bdb/env.c
> >  subversion/libsvn_ra_neon/session.c,
> >  subversion/libsvn_ra_serf/win32_auth_sspi.c,
> >  subversion/libsvn_ra_svn/cyrus_auth.c,
> >  subversion/libsvn_subr/io.c,
> >  subversion/libsvn_subr/sqlite.c,
> >  subversion/libsvn_subr/win32_xlate.c,
> >  subversion/svnserve/cyrus_auth.c:
> >    Update svn_atomic__init_once() calls and callbacks to pass/accept a
> baton.
>
> The pool parameter seems redundant now. Should a function require a
> pool, it can just pass it as the baton, or within the baton.
>

Are you saying that *any* function which takes a baton should not require a
pool parameter?  This doesn't jive when my interpretation.  We should still
provide a scratch pool for these types of functions, with all that such a
pool implies.  If the caller needs a permanent pool, then that pool goes
into the baton.

-Hyrum