On Tue, Apr 7, 2009 at 13:25, Stefan Sperling <stsp_at_elego.de> wrote:
> On Tue, Apr 07, 2009 at 07:15:10PM +0800, Edmund Wong wrote:
>> Hi,
>>
>> Again, reading svn_io.h, I came across a "TODO" for the
>> svn_stream_copy3() :
>>
>> * ### TODO: should close the streams ALWAYS, even on error exit
>>
>> Thinking that it might be my chance to do something for a
>> change, I looked into stream.c (this *is* the file, right?)
>> and came across the following:-
>>
>> svn_error_t *svn_stream_copy3(svn_stream_t *from, svn_stream_t *to,
>> svn_cancel_func_t cancel_func,
>> void *cancel_baton,
>> apr_pool_t *scratch_pool)
>> {
>> char *buf = apr_palloc(scratch_pool, SVN__STREAM_CHUNK_SIZE);
>>
>> /* Read and write chunks until we get a short read, indicating the
>> end of the stream. (We can't get a short write without an
>> associated error.) */
>> while (1)
>> {
>> apr_size_t len = SVN__STREAM_CHUNK_SIZE;
>>
>> if (cancel_func)
>> SVN_ERR(cancel_func(cancel_baton));
>>
>> SVN_ERR(svn_stream_read(from, buf, &len));
>> if (len > 0)
>> SVN_ERR(svn_stream_write(to, buf, &len));
>> if (len != SVN__STREAM_CHUNK_SIZE)
>> break;
>> }
>>
>>
>> return svn_error_compose_create(svn_stream_close(from),
>> svn_stream_close(to));
>> }
>>
>> Does this mean that since svn_stream_copy3() calls
>> svn_stream_close() on both from and to, both from
>> and to are closed after the return? (Though I'm not sure if
>> the resulting closure will result in an error.)
>>
>> If it does mean so, shouldn't the TODO be 'removed'?
>
> All the SVN_ERR(...) calls might return if an error is
> thrown from the function wrapped in SVN_ERR().
>
> So I'd say the TODO is still valid.
Right. I added to the TODO for specifically that reason: the streams
may not get closed if an error occurs. In some scenarios, we want to
ensure that svn_stream_close() is *always* called on certain streams.
Their logic needs/wants to rely on the fact that the streams will
always be closed after calling svn_stream_copy3().
Edmund: this should be a nice little bite-sized task to tweak that
function to ensure they're always closed.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1576194
Received on 2009-04-07 13:42:53 CEST