You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/02/08 19:58:31 UTC
Re: svn commit: r907414 - /subversion/trunk/subversion/libsvn_wc/questions.c
On Sun, Feb 7, 2010 at 06:50, <rh...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/questions.c Sun Feb 7 11:50:50 2010
> @@ -367,12 +367,15 @@
> return SVN_NO_ERROR;
> }
>
> + SVN_ERR(err);
> +
> /* Check all bytes, and verify checksum if requested. */
> - SVN_ERR(compare_and_verify(modified_p, db, local_abspath, pristine_stream,
> - compare_textbases, force_comparison,
> - scratch_pool));
> + err = compare_and_verify(modified_p, db, local_abspath, pristine_stream,
> + compare_textbases, force_comparison,
> + scratch_pool);
>
> - return SVN_NO_ERROR;
> + return svn_error_compose_create(err,
> + svn_stream_close(pristine_stream));
> }
It would be better to have compare_and_verify() close the stream after
it has done its work. Invariably, a stream on a file will not be used
once it hits EOF, so you may as well close it instead of waiting for
pool-cleanup.
compare_and_verify() is a local function, so it should be easy enough
to look at all uses, and ensure that closing the stream is acceptable.
If a (semi)public function passes one of its arguments into
compare_and_verify(), then you can use svn_stream_disown() before
passing that stream into compare_and_verify().
If a function that does that is semi-public, then you could even
examine all of its callers, and adjust the docstrings accordingly, to
talk about stream closure.
Cheers,
-g
Re: svn commit: r907414 -
/subversion/trunk/subversion/libsvn_wc/questions.c
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Feb 08, 2010 at 06:10:32PM -0500, Greg Stein wrote:
> On Mon, Feb 8, 2010 at 18:06, Stefan Sperling <st...@elego.de> wrote:
> > On Mon, Feb 08, 2010 at 02:58:31PM -0500, Greg Stein wrote:
> >> Invariably, a stream on a file will not be used
> >> once it hits EOF
> >
> > Unless it implements mark/seek or reset. See the patch code.
>
> Well aware of that. Read the reset of the thread :-P
Glad you've been reviewing :)
Stefan
Re: svn commit: r907414 - /subversion/trunk/subversion/libsvn_wc/questions.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Feb 8, 2010 at 18:06, Stefan Sperling <st...@elego.de> wrote:
> On Mon, Feb 08, 2010 at 02:58:31PM -0500, Greg Stein wrote:
>> Invariably, a stream on a file will not be used
>> once it hits EOF
>
> Unless it implements mark/seek or reset. See the patch code.
Well aware of that. Read the reset of the thread :-P
Re: svn commit: r907414 -
/subversion/trunk/subversion/libsvn_wc/questions.c
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Feb 08, 2010 at 02:58:31PM -0500, Greg Stein wrote:
> Invariably, a stream on a file will not be used
> once it hits EOF
Unless it implements mark/seek or reset. See the patch code.
Stefan