Libguestfs - Jan 2019 - [PATCH 2/2] python: change return type for RBufferOut with Python 3 (RHBZ#1661871)

A couple of fixes for Python 3 to the Python binding.

Unfortunately a behaviour change is needed, although it fixes broken
types used.

Pino Toscano (2):
  python: fix call of Python handlers of events
  python: change return type for RBufferOut with Python 3 (RHBZ#1661871)

 generator/python.ml | 4 ++++
 python/handle.c     | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.20.1

Make sure to reference the arguments, to make sure they are kept alive
during the function call; this is visible when setting an event handler
for the CLOSE event, and testing it with Python 3.

This does not seem to create a memory leak e.g. with Python 2.

Also, switch away from the quasi-internal PyEval_CallObject to the
public PyObject_CallObject, which takes care of doing safety checks.
---
 python/handle.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/python/handle.c b/python/handle.c
index e84a04195..adba1b823 100644
--- a/python/handle.c
+++ b/python/handle.c
@@ -136,11 +136,12 @@ guestfs_int_py_event_callback_wrapper (guestfs_h *g,
   args = Py_BuildValue ("(Kis#O)",
                         (unsigned PY_LONG_LONG) event, event_handle,
                         buf, buf_len, py_array);
+  Py_INCREF (args);
 
   if (PyEval_ThreadsInitialized ())
     py_save = PyGILState_Ensure ();
 
-  py_r = PyEval_CallObject (py_callback, args);
+  py_r = PyObject_CallObject (py_callback, args);
 
   if (PyEval_ThreadsInitialized ())
     PyGILState_Release (py_save);
-- 
2.20.1

So far RBufferOut return values are 'str' on all the versions of Python.
Python 3 distinguishes between 'str' (unicode strings), and
'bytes',
with 'str' no more able to hold arbitrary data.

For this reason, switch the return value of RBufferOut functions to
bytes on Python 3: while this is a potentially incompatibile change,
this is the only way to handle safely sequences of arbitrary bytes, like
all the ones returned by RBufferOut functions.
---
 generator/python.ml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/generator/python.ml b/generator/python.ml
index a70faec8c..c10adcb69 100644
--- a/generator/python.ml
+++ b/generator/python.ml
@@ -511,7 +511,11 @@ and generate_python_actions actions ()             pr
"  guestfs_int_free_string_list (r);\n";
            pr "  if (py_r == NULL) goto out;\n";
        | RBufferOut _ ->
+           pr "#if PY_MAJOR_VERSION >= 3\n";
+           pr "  py_r = PyBytes_FromStringAndSize (r, size);\n";
+           pr "#else\n";
            pr "  py_r = guestfs_int_py_fromstringsize (r, size);\n";
+           pr "#endif\n";
            pr "  free (r);\n";
            pr "  if (py_r == NULL) goto out;\n";
       );
-- 
2.20.1

On Tue, Jan 22, 2019 at 02:00:22PM +0100, Pino Toscano
wrote:
> So far RBufferOut return values are 'str' on all the versions of
Python.
> Python 3 distinguishes between 'str' (unicode strings), and
'bytes',
> with 'str' no more able to hold arbitrary data.
> 
> For this reason, switch the return value of RBufferOut functions to
> bytes on Python 3: while this is a potentially incompatibile change,
> this is the only way to handle safely sequences of arbitrary bytes, like
> all the ones returned by RBufferOut functions.
> ---
>  generator/python.ml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/generator/python.ml b/generator/python.ml
> index a70faec8c..c10adcb69 100644
> --- a/generator/python.ml
> +++ b/generator/python.ml
> @@ -511,7 +511,11 @@ and generate_python_actions actions () >           
pr "  guestfs_int_free_string_list (r);\n";
>             pr "  if (py_r == NULL) goto out;\n";
>         | RBufferOut _ ->
> +           pr "#if PY_MAJOR_VERSION >= 3\n";
> +           pr "  py_r = PyBytes_FromStringAndSize (r, size);\n";
> +           pr "#else\n";
>             pr "  py_r = guestfs_int_py_fromstringsize (r,
size);\n";
> +           pr "#endif\n";
>             pr "  free (r);\n";
>             pr "  if (py_r == NULL) goto out;\n";
>        );
> -- 
ACK series.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org