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

Re: [PATCH] swig-py svn_stream_t read() glue

From: Troy Curtis Jr <troycurtisjr_at_gmail.com>
Date: Sun, 07 Jan 2018 15:17:17 +0000

On Wed, Jan 3, 2018 at 10:55 PM Daniel Shahaf <d.s_at_daniel.shahaf.name>
wrote:

> svn_swig_py_make_stream() is a function that wraps a PyObject in an
> svn_stream_t. Its read implementation, read_handler_pyio(), just calls
> the PyObject's .read() method.
>
> According to <https://docs.python.org/2/library/io.html#io.RawIOBase.read
> >,
> the read() method of "raw" file-like objects makes only one read(2)
> syscall, and so may return fewer bytes than were requested. (The py3
> docs are similar.)
>
>
Probably in practice the passed in objects are one of the buffered types
where the read() behaves as this code obviously expected, which is likely
why this hasn't been noticed before. However, we can't actually *know*
that, and anything with a read() method would actually be perfectly valid.
So it seems like a good patch to me.

However, svn_stream_t "full read" functions are obligated to return the
> number of bytes requested, unless EOF.
>
> Therefore, do we need the following patch?
>
> [[[
> swig-py: Support raw binary file-like objects for readable svn_stream_t*
> parameters. [D:bindings]
>
> * subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
> (svn_swig_py_make_stream): Declare read_handler_pyio() as a non-full
> svn_read_fn_t, in case PY_IO is a raw binary file object.
> ]]]
>
> [[[
> Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
> ===================================================================
> --- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
> (revision 1819751)
> +++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
> (working copy)
> @@ -2578,8 +2578,7 @@ svn_swig_py_make_stream(PyObject *py_io, apr_pool_
> svn_stream_t *stream;
>
> stream = svn_stream_create(py_io, pool);
> - svn_stream_set_read2(stream, NULL /* only full read support */,
> - read_handler_pyio);
> + svn_stream_set_read2(stream, read_handler_pyio, NULL);
> svn_stream_set_write(stream, write_handler_pyio);
> svn_stream_set_close(stream, close_handler_pyio);
> apr_pool_cleanup_register(pool, py_io, svn_swig_py_stream_destroy,
> ]]]
>
> Cheers,
>
> Daniel
>
Received on 2018-01-07 16:17:35 CET

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