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 <bert_at_qqmail.nl> wrote:
> The called function does an explicit disown so i assumed that behavior was by design.
>
> Bert
>
> ----- Oorspronkelijk bericht -----
> Van: Greg Stein <gstein_at_gmail.com>
> Verzonden: maandag 8 februari 2010 20:58
> Aan: dev_at_subversion.apache.org
> Onderwerp: Re: svn commit: r907414 - /subversion/trunk/subversion/libsvn_wc/questions.c
>
> 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 21:31:16 CET