You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2010/02/07 12:50:50 UTC
svn commit: r907414 - /subversion/trunk/subversion/libsvn_wc/questions.c
Author: rhuijben
Date: Sun Feb 7 11:50:50 2010
New Revision: 907414
URL: http://svn.apache.org/viewvc?rev=907414&view=rev
Log:
Following up on r89406 and r898492, resolve several Windows test failures
by closing files after usage. Also remove an error leak introduced in
r898492.
* subversion/libsvn_wc/questions.c
(svn_wc__internal_text_modified_p): Close pristine stream and clear other
errors than ENOENT.
Modified:
subversion/trunk/subversion/libsvn_wc/questions.c
Modified: subversion/trunk/subversion/libsvn_wc/questions.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/questions.c?rev=907414&r1=907413&r2=907414&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/questions.c (original)
+++ 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));
}
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
Re: svn commit: r907414 - /subversion/trunk/subversion/libsvn_wc/questions.c
Posted by Greg Stein <gs...@gmail.com>.
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