Re: svn commit: r1072519 - Add svn_stream_skip(), svn_stream_buffered(), svn_stream_supports_mark()

On Wed, 2011-06-01 at 15:08 +0100, Julian Foad wrote:
> On Sun, 2011-02-20, stefan2_at_apache.org wrote:
> > Merge all changes (r1068695 - r1072516) from the
> > integrate-stream-api-extensions.
> >
> > These patches add svn_stream_skip(), svn_stream_buffered()
> > and svn_stream_supports_mark().
>
> A belated review.

And some more.

> svn_error_t *
> svn_stream_skip(svn_stream_t *stream, apr_size_t *len)
> {
> SVN_ERR_ASSERT(stream->skip_fn != NULL);

This should call the default skip implementation (which reads and
discards) when a stream doesn't implement its own skip function. That
way, 'skip' will be available on any generic stream, which is better
than the caller having to know (either implicitly or by calling a test
function) whether the stream has a skip function installed.

> return stream->skip_fn(stream->baton, len);
> }

> static svn_error_t *
> skip_handler_empty(void *baton, apr_size_t *count)
> {
> *count = 0;
> return SVN_NO_ERROR;
> }

I thought a skip handler was supposed to return an error when asked to
read past EOF. This one doesn't. Anyway, if we make the above change
then we can get rid of this handler; the empty stream doesn't need one.

> static svn_boolean_t
> is_buffered_handler_empty(void *baton)
> {
> return FALSE;
> }

We don't need this handler, because there's a default for streams that
don't supply a handler.

- Julian
Received on 2011-06-01 17:07:10 CEST