> -----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
>
> Author: stefan2
> Date: Thu Feb 10 00:35:35 2011
> New Revision: 1069176
>
> URL: http://svn.apache.org/viewvc?rev=1069176&view=rev
> Log:
> Incorporate review feedback from Blair (see
> http://svn.haxx.se/dev/archive-2011-02/0259.shtml).
>
> * subversion/include/svn_io.h
> (svn_stream_skip): Explain the (non-)semantics of the skip operation
> * subversion/libsvn_subr/subst.c
> (translated_stream_skip): fix implementation as suggested
> * subversion/libsvn_subr/stream.c
> (skip_default_handler, skip_handler_apr): dito
> (buffered_handler_apr): use apr_file_flags_get instead of
> apr_file_buffer_size_get to become compatible with APR 0.9
>
> Found by: blair
>
> Modified:
> subversion/branches/integrate-stream-api-
> extensions/subversion/include/svn_io.h
> subversion/branches/integrate-stream-api-
> extensions/subversion/libsvn_subr/stream.c
> subversion/branches/integrate-stream-api-
> extensions/subversion/libsvn_subr/subst.c
>
> Modified: subversion/branches/integrate-stream-api-
> extensions/subversion/include/svn_io.h
> URL: http://svn.apache.org/viewvc/subversion/branches/integrate-stream-
> api-
> extensions/subversion/include/svn_io.h?rev=1069176&r1=1069175&r2=106917
> 6&view=diff
> =======================================================================
> =======
> --- subversion/branches/integrate-stream-api-
> extensions/subversion/include/svn_io.h (original)
> +++ subversion/branches/integrate-stream-api-
> extensions/subversion/include/svn_io.h Thu Feb 10 00:35:35 2011
> @@ -1047,7 +1047,17 @@ svn_stream_read(svn_stream_t *stream,
> char *buffer,
> apr_size_t *len);
>
> -/** Skip data from a generic stream. @see svn_stream_t. */
> +/**
> + * Skip COUNT bytes from a generic STREAM. If the stream is exhausted
> + * before COUNT bytes have been read, an error will be returned and
> + * COUNT will be changed to the actual number of bytes skipped.
> + *
> + * NOTE. No assumption can be made on the semantics of this function
> + * other than that the stream read pointer will be advanced by *count
> + * bytes. Depending on the capabilities of the underlying stream
> + * implementation, this may for instance be translated into a sequence
> + * of reads or a simple seek operation.
> + */
> svn_error_t *
> svn_stream_skip(svn_stream_t *stream,
> apr_size_t *count);
>
> Modified: subversion/branches/integrate-stream-api-
> extensions/subversion/libsvn_subr/stream.c
> URL: http://svn.apache.org/viewvc/subversion/branches/integrate-stream-
> api-
> extensions/subversion/libsvn_subr/stream.c?rev=1069176&r1=1069175&r2=10
> 69176&view=diff
> =======================================================================
> =======
> --- 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, ¤t, 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.
Maybe you should set *count to 0 before calling seek?
But fixing the documentation to not promise a valid output on error is probably an easier option.
Bert
Received on 2011-02-10 11:04:24 CET