On Thu, 2010-03-18, rhuijben_at_apache.org wrote:
> Author: rhuijben
> Date: Thu Mar 18 14:21:26 2010
> New Revision: 924797
>
> URL: http://svn.apache.org/viewvc?rev=924797&view=rev
> Log:
> Extract the range handling from the common apr read codepath in the streams
> api, as there is no need to complicate this common code path here just
> for being able to read from a specific range.
Good.
> * subversion/libsvn_subr/stream.c
> (read_handler_apr): Extract the range code and move it to ...
> (read_range_handler_apr): ... this new function wrapping read_handler_apr.
> (svn_stream_from_aprfile_range_readonly): Hook read_range_handler_apr.
> Add note about handling disown properly.
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/stream.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/stream.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=924797&r1=924796&r2=924797&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/stream.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/stream.c Thu Mar 18 14:21:26 2010
> @@ -676,35 +676,6 @@ read_handler_apr(void *baton, char *buff
> struct baton_apr *btn = baton;
> svn_error_t *err;
>
> - /* Check for range restriction. */
> - if (btn->start >= 0 && btn->end > 0)
> - {
> - /* Get the current file position and make sure it is in range. */
> - apr_off_t pos;
> -
> - pos = 0;
> - SVN_ERR(svn_io_file_seek(btn->file, APR_CUR, &pos, btn->pool));
> - if (pos < btn->start)
> - {
> - /* We're before the range, so forward the file cursor to
> - * the start of the range. */
> - pos = btn->start;
> - SVN_ERR(svn_io_file_seek(btn->file, APR_SET, &pos, btn->pool));
> - }
> - else if (pos >= btn->end)
> - {
> - /* We're past the range, indicate that no bytes can be read. */
> - *len = 0;
> - return SVN_NO_ERROR;
> - }
> - else
> - {
> - /* We're in range, but don't read over the end of the range. */
> - if (pos + *len > btn->end)
> - *len = btn->end - pos;
> - }
> - }
> -
> err = svn_io_file_read_full(btn->file, buffer, *len, len, btn->pool);
> if (err && APR_STATUS_IS_EOF(err->apr_err))
> {
[...]
> +static svn_error_t *
> +read_range_handler_apr(void *baton, char *buffer, apr_size_t *len)
Please can we all get into the habit of putting a doc string on EVERY
new function, even when it does not seem like there is much that needs
to be said about it.
> {
> - return svn_stream_from_aprfile2(file, TRUE, pool);
> + struct baton_apr *btn = baton;
> +
> + /* ### BH: I think this can be simplified/optimized by just keeping
> + track of the current position. */
> +
> + /* Check for range restriction. */
> + if (btn->start >= 0 && btn->end > 0)
> + {
> + /* Get the current file position and make sure it is in range. */
> + apr_off_t pos;
> +
> + pos = 0;
> + SVN_ERR(svn_io_file_seek(btn->file, APR_CUR, &pos, btn->pool));
> + if (pos < btn->start)
> + {
> + /* We're before the range, so forward the file cursor to
> + * the start of the range. */
> + pos = btn->start;
> + SVN_ERR(svn_io_file_seek(btn->file, APR_SET, &pos, btn->pool));
> + }
> + else if (pos >= btn->end)
> + {
> + /* We're past the range, indicate that no bytes can be read. */
> + *len = 0;
> + return SVN_NO_ERROR;
> + }
> + else
> + {
> + /* We're in range, but don't read over the end of the range. */
> + if (pos + *len > btn->end)
> + *len = btn->end - pos;
> + }
I think that last block needs to take effect even if 'pos' was
previously before 'start'. Otherwise there can be a too-long read.
(I know you only moved this code.)
- Julian
> + }
> +
> + return read_handler_apr(baton, buffer, len);
> }
>
> +
> svn_stream_t *
> svn_stream_from_aprfile_range_readonly(apr_file_t *file,
> svn_boolean_t disown,
> @@ -863,6 +869,7 @@ svn_stream_from_aprfile_range_readonly(a
> svn_stream_t *stream;
> apr_off_t pos;
>
> + /* ### HACK: These shortcuts don't handle disown FALSE properly */
> if (file == NULL || start < 0 || end <= 0 || start >= end)
> return svn_stream_empty(pool);
>
> @@ -877,7 +884,7 @@ svn_stream_from_aprfile_range_readonly(a
> baton->start = start;
> baton->end = end;
> stream = svn_stream_create(baton, pool);
> - svn_stream_set_read(stream, read_handler_apr);
> + svn_stream_set_read(stream, read_range_handler_apr);
> svn_stream_set_reset(stream, reset_handler_apr);
> svn_stream_set_mark(stream, mark_handler_apr);
> svn_stream_set_seek(stream, seek_handler_apr);
>
>
Received on 2010-03-18 17:22:41 CET