subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <>
Subject Re: svn commit: r1701633 - /subversion/trunk/subversion/libsvn_subr/stream.c
Date Tue, 08 Sep 2015 11:29:26 GMT
On 8 September 2015 at 14:10, Evgeny Kotkov <> wrote:
> Ivan Zhakov <> 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?
Looks good for me.

> 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.
Agree that this can be improved separately.

Ivan Zhakov

View raw message