Re: svn commit: r1069176 - in /subversion/branches/integrate-stream-api-extensions/subversion: include/svn_io.h libsvn_subr/stream.c libsvn_subr/subst.c

On 10.02.2011 11:03, Bert Huijben wrote:
>
>> -----Original Message-----
>> From: stefan2_at_apache.org [mailto:stefan2_at_apache.org]
>> Sent: donderdag 10 februari 2011 1:36
>> To: commits_at_subversion.apache.org
>> Subject: svn commit: r1069176 - in /subversion/branches/integrate-
>> stream-api-extensions/subversion: include/svn_io.h libsvn_subr/stream.c
>> libsvn_subr/subst.c

>> --- subversion/branches/integrate-stream-api-
>> extensions/subversion/libsvn_subr/stream.c (original)
>> +++ subversion/branches/integrate-stream-api-
>> extensions/subversion/libsvn_subr/stream.c Thu Feb 10 00:35:35 2011
>> @@ -456,12 +456,14 @@ skip_default_handler(void *baton, apr_si
>> apr_size_t bytes_read;
>> char buffer[4096];
>> svn_error_t *err = SVN_NO_ERROR;
>> + apr_size_t to_read = *count;
>>
>> - while ((total_bytes_read< *count)&& !err)
>> + while ((to_read> 0)&& !err)
>> {
>> - bytes_read = sizeof(buffer)< *count ? sizeof(buffer) : *count;
>> + bytes_read = sizeof(buffer)< to_read ? sizeof(buffer) :
>> to_read;
>> err = read_fn(baton, buffer,&bytes_read);
>> total_bytes_read += bytes_read;
>> + to_read -= bytes_read;
>> }
>>
>> *count = total_bytes_read;
>> @@ -673,9 +675,11 @@ skip_handler_apr(void *baton, apr_size_t
>> {
>> struct baton_apr *btn = baton;
>> apr_off_t offset = *count;
>> + apr_off_t current = 0;
>>
>> - SVN_ERR(svn_io_file_seek(btn->file, SEEK_CUR,&offset, btn->pool));
>> - *count = offset;
>> + SVN_ERR(svn_io_file_seek(btn->file, APR_CUR,&current, btn->pool));
>> + SVN_ERR(svn_io_file_seek(btn->file, APR_CUR,&offset, btn->pool));
>> + *count = offset - current;
> The documentation says an error is returned *and* count is set. The implementation does one or the other. If svn_io_file_seek fails SVN_ERR() will return before setting *count.
>
Good point.
> Maybe you should set *count to 0 before calling seek?
Yep.
> But fixing the documentation to not promise a valid output on error is probably an easier option.
That would make it somewhat inconsistent with the
"reading" implementations of skip() for other streams.
Also, we should try to transport as much information
from the underlying APR layer as possible.

-- Stefan^2.
Received on 2011-02-11 01:51:15 CET