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

Ivan Zhakov <ivan_at_visualsvn.com> writes:

> It may be worth to introduce local helper like
> 'make_stream_from_apr_file(svn_boolean_t supports_seek)' and use it in
> svn_stream_from_apr_file2() and svn_stream_for_stdin() instead of
> clearing mark/seek handlers after calling svn_stream_from_apr_file2().

Sounds good.

> Another problem that currently svn_stream_t for stdin/stderr/stdout
> pretends to support native svn_stream_skip(), while I'm not sure that
> all platforms support seek for stdin.

Indeed, I missed that default skip_handler_apr() performs a seek that could
be unavailable or behave surprisingly when used with a handle like STDIN.
So, the fix is partial.

What do you think about the attached patch?

As of other callbacks, I found out that data_available_handler_apr() calls
PeekNamedPipe() on Windows, and if the STDIN handle is a tty, this call
errors out with ERROR_INCORRECT_FUNCTION. We currently wrap all errors
originating from PeekNamedPipe() into SVN_ERR_STREAM_NOT_SUPPORTED, so,
technically, this behavior follows the contract. Still, I am thinking
that this is something we could improve separately.

Regards,
Evgeny Kotkov