Philip Martin <philip@codematters.co.uk> writes:
> > + /* Make sure our entry exists in the parent. */
> > + {
> > + svn_wc_adm_access_t *paccess;
> > +
> > + svn_path_split (svn_wc_adm_access_path (loggy->adm_access), &pdir,
> > + &base_name, pool);
> > +
> > + err = svn_wc_adm_retrieve (&paccess, loggy->adm_access, pdir, pool);
> > + if (err && (err->apr_err == SVN_ERR_WC_NOT_LOCKED))
> > + SVN_ERR (svn_wc_adm_open (&paccess, NULL, pdir, TRUE, FALSE, pool));
>
> This leaks the error err.
Thank you -- dang, I always forget that.
> The associated baton is NULL, and the returned baton is not explicitly
> closed, thus the lock only gets removed by a pool cleanup handler. Is
> this intentional? Not explicitly closing write access batons has
> cause me a problem in the past.
What I'm worried about is the case where the baton was retrieved from
the associated set. We don't want to close it in that case, I think,
we just want to leave it where we found it. But if we got it via
_open(), then we could close it.
Is it worth adding a boolean and a check at the end of the code? I
decided to just depend on the pool eventually being cleared, but now
that you mention it, the code could fail if multiple entities are
added in that parent directory.
Should the baton be unconditionally added to the associated set? That
seems like an unwise side effect of the parent-updating code...
?
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 22 01:49:57 2003