Re: svn commit: rev 7468 - in trunk/subversion: libsvn_ra_dav mod_dav_svn

On Tue, Oct 21, 2003 at 04:05:29PM -0500, kfogel@tigris.org wrote:
>...
> +++ trunk/subversion/libsvn_ra_dav/fetch.c Tue Oct 21 16:05:28 2003
> @@ -2042,6 +2042,9 @@
> const char *vcc;
> int http_status;
>
> + SVN_ERR (svn_ra_dav__get_vcc
> + (&vcc, rb->ras->sess, rb->ras->url, rb->ras->pool));
>...
> - err = svn_ra_dav__get_vcc(&vcc, rb->ras->sess, rb->ras->url, rb->ras->pool);
> - if (err)
> - {
> - (void) apr_file_close(rb->tmpfile);
> - return err;
> - }

Previously, a failure in getting the VCC would close the file. You altered
the behavior here -- it doesn't close that file before throwing an error.

While it might not be a problem, in practice (it will eventually be closed
by a pool clear/destory), I don't think the postcondition of this function
should be altered.

If you *do* want to keep the SVN_ERR() because (I believe) it will always
throw the error "far enough" to toss the pool and the file, then I'd
suggest altering the code just after that line. That code closes the file
before returning. IOW, before your change, the consistent approach was
"close before return". "close by pool" is probably valid, but the two
error/return conditions should be sync'd.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org