On 8 September 2015 at 14:10, Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com> wrote:
> 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?
>
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
Received on 2015-09-08 13:30:08 CEST