subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Evgeny Kotkov <evgeny.kot...@visualsvn.com>
Subject Re: svn commit: r1702305 - /subversion/trunk/subversion/libsvn_subr/stream.c
Date Thu, 10 Sep 2015 21:50:07 GMT
Stefan Fuhrmann <stefan2@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].

[1] https://svn.apache.org/repos/asf/apr/apr/branches/1.3.x/file_io/unix/open.c
[2] https://svn.apache.org/repos/asf/apr/apr/trunk/file_io/unix/pipe.c
[3] https://svn.apache.org/repos/asf/apr/apr/trunk/file_io/win32/pipe.c


Regards,
Evgeny Kotkov

Mime
View raw message