[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Stefan Fuhrmann <stefanfuhrmann_at_alice-dsl.de>
Date: Thu, 02 Jun 2011 00:34:18 +0200

On 01.06.2011 17:06, Julian Foad wrote:
> 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.
>
>
Thanks for reviewing!
>> 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.
Agreed.
>> 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.
+1. This function is unlikely to be called for an empty
  stream anyways.

>> 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.
Back then, I had just discovered that translated streams (?)
did not provide a specialized skip() implementation. That
prompted me to look for any "missing" handler.

But this one is indeed not needed.

-- Stefan^2.
Received on 2011-06-02 00:34:51 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.