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...
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 */ ?
Stefan
---------------------------------------------------------------------
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 19:58:50 CEST