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

Re: svn commit: r1702305 - /subversion/trunk/subversion/libsvn_subr/stream.c

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Fri, 11 Sep 2015 11:35:39 +0200

On Fri, Sep 11, 2015 at 11:05 AM, Stefan Fuhrmann <
stefan.fuhrmann_at_wandisco.com> wrote:

> On Fri, Sep 11, 2015 at 11:01 AM, Branko Čibej <brane_at_apache.org> wrote:
>
>> On 10.09.2015 23:50, Evgeny Kotkov wrote:
>> > Stefan Fuhrmann <stefan2_at_apache.org> writes:
>> >
>> > + * Special files like STDIN will have no flags set at all. In that
>> case,
>> > + * we can't filter and must allow any operation - which may then
>> fail at
>> > + * the APR level further down the stack.
>> > + */
>> > + apr_uint32_t flags = apr_file_flags_get(file);
>> > + svn_boolean_t supports_read = (flags == 0) || (flags & APR_READ);
>> > + svn_boolean_t supports_write = (flags == 0) || (flags & APR_WRITE);
>> >
>> > Files corresponding to the standard I/O streams actually have the
>> appropriate
>> > APR_READ / APR_WRITE flags set starting from APR 1.3 — see, for example,
>> > apr_file_open_flags_stderr() in [1]. Hence, the (flags == 0) check is
>> going
>> > to return false for them.
>> >
>> >> Note that this check is not perfect for arbitrary APR file handles
>> >> (may enable more functions than the handle actually supports) but
>> >> works correctly for our STD* streams and the files opened through
>> >> our svn_io_* API.
>> > The actual problem, to my mind, is that relying on flags to determine
>> if the
>> > file allows reading or writing, is fragile. There are examples of
>> apr_file_t's
>> > that don't have the corresponding flags, but still allow reading and
>> writing,
>> > and svn_stream_from_aprfile2() is going to break for them.
>> >
>> > One example would be apr_file_pipe_create() on Unix that sets
>> APR_INHERIT
>> > flag on the created pipe [2]. Another example is creating a pipe on
>> Windows
>> > that currently initializes flags to zero [3].
>>
>> I've reviewed the JavaHL code again and it indeed appears that this is
>> caused by the difference in implementations of apr_file_pipe_create_ex()
>> on Windows and elsewhere. The bindings code itself is not
>> platform-specific; see TunnelContext in
>> subversion/bindings/javahl/native/OperationContext.cpp. Those pipes get
>> wrapped into streams deep in the RA layer.
>>
>> One could argue that apr_file_pipe_create is broken since it doesn't set
>> the appropriate flags on the input and output handles, but that doesn't
>> really help make this particular instance in the bindings code work.
>>
>
> O.k. Then I'll simply revert.
>

Done in r1702410.

-- Stefan^2.
Received on 2015-09-11 11:35:48 CEST

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