On Sun, Feb 7, 2010 at 06:50, <rhuijben_at_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
Received on 2010-02-08 20:59:05 CET