Re: Error leak in libsvn_client/revert.c

"Max Bowsher" <maxb@ukf.net> writes:

> ==========================================================
> err = svn_wc_revert2 (path, adm_access, recursive, use_commit_times,
> ctx->cancel_func, ctx->cancel_baton,
> ctx->notify_func2, ctx->notify_baton2,
> pool);
>
> /* If no error, or SVN_ERR_UNVERSIONED_RESOURCE error, then we want
> to close up before returning. For any other kind of error, we
> want to leave things exactly as they were when the error
> occurred. */
>
> if (err && err->apr_err != SVN_ERR_UNVERSIONED_RESOURCE)
> return err;
>
> SVN_ERR (svn_wc_adm_close (adm_access));
>
> return err;
> ==========================================================
>
> Potential error leak if svn_wc_revert2 errors, and then
> svn_wc_adm_close errors too.
>
> I don't understand that comment about when to close the adm_access and
> when not, so I'm not sure how to fix it correctly.

I think an SVN_ERR_UNVERSIONED_RESOURCE error is "acceptable" as it
indicates an unversioned item. Since the error is acceptable it
should not cause the baton to be left open. So probably something
like:

   if (err && err->apr_err != SVN_ERR_UNVERSIONED_RESOURCE)
     return err;

   err2 = svn_wc_adm_close (adm_access);
   if (err2 && ! err)
     return err2
   svn_error_clear (err2);

   return err;

It might be more elegant to call the notification function in revert
instead of svn_client_revert.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org