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

Re: svn commit: r1561688 - in /subversion/trunk/subversion: include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/stream.c

From: Branko Čibej <brane_at_wandisco.com>
Date: Tue, 28 Jan 2014 11:56:04 +0100

On 27.01.2014 15:16, rhuijben_at_apache.org wrote:
> Author: rhuijben
> Date: Mon Jan 27 14:16:08 2014
> New Revision: 1561688
>
> URL: http://svn.apache.org/r1561688
> Log:
> Following up on r1561387, which was (temporarily) reverted in r1561424 extend
> the existing stream api with an optional incomplete read function to allow
> using the stream api as was intended by r1561387.
>
> After discussion on irc we came with the idea of revving svn_stream_read()
> to two separate functions: one with full read semantics (like before) and one
> with the new possibly incomplete read support.
>
> This patch currently provides a default implementation of the incomplete read
> with a forced complete read. Brane suggests that it might be better to just
> return an error for requested incomplete reads. I commit this version under
> the assumption that it is very easy to change this later in a separate patch.

I'll explain why ...

> +void
> +svn_stream_set_read(svn_stream_t *stream,
> + svn_read_fn_t read_fn)
> +{
> + svn_stream_set_read2(stream, read_fn, read_fn);
> +}

Users of the deprecated svn_stream_set_read will presumably make read_fn
conform to the full-read semantics. If we set a full-read handler as the
partial-read, some other user of the stream could cal svn_stream_read2,
expecting to get a partial read but getting a full read; which could
lead to hard-to-diagnose deadlocks.

It is IMO much better to change the implementation to this:

  svn_stream_set_read2(stream, NULL, read_fn);

so that calling svn_stream_read2 on that stream (or a stream that wraps
it, possibly in a way not visible to the consumer) will cause an
assertion, which is a lot easier to deal with than random deadlocks.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane_at_wandisco.com
Received on 2014-01-28 11:56:48 CET

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