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