Sounds silly. I'd suggest removing that, and doc'ing and explicit closure. *Any* function that reads a stream to EOF should just close the darned stream. (I adjusted some svn_io.h functions to do this) ... it just doesn't make sense to leave the stream open since you can't really do anything with a stream sitting at EOF. (sure, a few streams are (now) seekable, but fine... disown those before calling a close-on-EOF function)
Cheers, -g On Mon, Feb 8, 2010 at 15:22, Bert Huijben <b...@qqmail.nl> wrote: > The called function does an explicit disown so i assumed that behavior was by > design. > > Bert > > ----- Oorspronkelijk bericht ----- > Van: Greg Stein <gst...@gmail.com> > Verzonden: maandag 8 februari 2010 20:58 > Aan: dev@subversion.apache.org > Onderwerp: Re: svn commit: r907414 - > /subversion/trunk/subversion/libsvn_wc/questions.c > > On Sun, Feb 7, 2010 at 06:50, <rhuij...@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 > >
