> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: vrijdag 15 augustus 2008 19:58
> To: dev_at_subversion.tigris.org
> Subject: Re: [Patch] Make neon initialization thread safe
>
> On Fri, Aug 15, 2008 at 01:31:33PM -0400, Karl Fogel wrote:
> > "Bert Huijben" <B.Huijben_at_competence.biz> writes:
> > > Some users think SharpSvn is a nice api to make parallel requests
> to
> > > subversion servers. I received some crash reports lately and this
> patch
> > > resolves at least the crash I could reproduce in a testcase.
> > >
> > > This patch fixes a race condition if two neon requests are started
> > > approximately at the same time. The first call starts initializing
> and
> > > the second call thinks initialization is already completed.
> >
> > This patch looks good to me (I'm running 'make check' on it right
> now).
> > But I have one minor question:
> >
> > > --- subversion/libsvn_ra_neon/session.c (revision 32490)
> > > +++ subversion/libsvn_ra_neon/session.c (working copy)
> > > @@ -962,6 +965,25 @@
> > > }
> > >
> > > static svn_error_t *
> > > +initialize_neon(apr_pool_t* pool)
> > > +{
> > > + /* pool = NULL */
> > > + if (ne_sock_init() != 0)
> > > + return svn_error_create(SVN_ERR_RA_DAV_SOCK_INIT, NULL,
> > > + _("Network socket initialization
> failed"));
> > > +
> > > + return SVN_NO_ERROR;
> > > +}
> >
> > Why the comment "/* pool = NULL */" ? Wouldn't it be more accurate
> to
> > say that pool is ignored? It doesn't matter whether it's NULL or
> not;
> > we're not using the value anyway...
I tried to make sure we weren't going to use it later on.
> Since it's a static function, couldn't the parameter simply be
> removed altogether? Or does it implement some callback type which
> requires this argument? If so, why is there no comment above it
> that says /* An implementation of ..._t */ ?
It implements svn_error_t *(*init_func)(apr_pool_t*) as there is no
predefined svn_atomic_initialize_t type in svn_atomic.h.
(The pool value is only passed transparently to the initializer, but there
is no pool variable available with the right lifetime; the same case as in
the sasl_auth initialization).
Thanks!
Bert
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-15 21:19:45 CEST