similar to: [PATCH] python: Avoid leaking py_array along error paths

On Thu, Feb 16, 2023 at 03:23:51PM +0000, Richard W.M. Jones wrote: > See-also: https://listman.redhat.com/archives/libguestfs/2023-February/030730.html > Fixes: commit 6ef5837e2d8c5d4d83eff51c0201eb2e08f719de > Thanks: Laszlo Ersek > --- > python/handle.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/python/handle.c b/python/handle.c > index

Version 1 was here: https://listman.redhat.com/archives/libguestfs/2023-February/030732.html (Ignore version 2 which had a mistake, this is version 3) Following Eric's suggestion here: https://listman.redhat.com/archives/libguestfs/2023-February/030746.html let's decrement the reference of py_array right after adding it to the args. (This works even if args fails to be built).

Version 1 was here: https://listman.redhat.com/archives/libguestfs/2023-February/030732.html Following Eric's suggestion here: https://listman.redhat.com/archives/libguestfs/2023-February/030746.html let's decrement the reference of py_array right after adding it to the args. (This works even if args fails to be built). However the other part of Eric's suggestion is wrong as

In the case that building the parameters to the Python event callback fails, args was returned as NULL. We immediately tried to call Py_INCREF on this which crashed. Returning NULL means the Python function threw an exception, so print the exception and return (there is no way to return an error here - the event is lost). Reported-by: Yonatan Shtarkman See:

The event callback gets a buffer parameter which is usually something like a log message. However as it comes from C it is not necessarily well-formed (eg) UTF-8 but could contain any old byte sequence. In the test case provided by the reported, we failed to encode the buffer as str with this error: UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc3 in position 137: unexpected

On Thu, Feb 16, 2023 at 03:09:02PM +0100, Laszlo Ersek wrote: > On 2/14/23 19:51, Richard W.M. Jones wrote: > > In the case that building the parameters to the Python event callback > > fails, args was returned as NULL. We immediately tried to call > > Py_INCREF on this which crashed. Returning NULL means the Python > > function threw an exception, so print the

All Py_* functions should be protected by the GIL. Otherwise internal python data structures can get corrupted. Move PyGILState_Ensure to the beginning of the block and PyGILState_Release to the bottom. Signed-off-by: Sam Eiderman <sameid@google.com> --- python/handle.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/python/handle.c b/python/handle.c

The tracebacks are compressed into a single line because we're using PyObject_Str, but they are just about usable if not very readable. For example you would see an error like this: nbdkit: error: ./python-exception.py: config_complete: error: ['Traceback (most recent call last):\n', ' File "./python-exception.py", line 54, in config_complete\n raise_error1()\n',

Posting now that I got something to compile (at the expense of breaking OCaml bindings), but I'm open to ideas on how to improve it. Eric Blake (2): generator: Tweak print_c_arg_list to take alternate first arg RFC: generator: Handle shared callbacks in Python generator/generator | 556 ++++++++++++++++++++++---------------------- 1 file changed, 280 insertions(+), 276 deletions(-) --

v1 was here: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00231 The changes address everything that Eric picked up in his review of the first two patches. I have also added two more patches (4 and 5) which respectively fix docs and change int status -> unsigned status, as discussed. Passes make, check, check-valgrind. Rich.

A couple of fixes for Python 3 to the Python binding. Unfortunately a behaviour change is needed, although it fixes broken types used. Changes from v1: - handle also FBuffer in structs Pino Toscano (2): python: fix call of Python handlers of events python: change types for RBufferOut/FBuffer with Python 3 (RHBZ#1661871) generator/python.ml | 9 +++++++++ python/handle.c | 3 ++- 2

Patches 1-4 are basically uncontroversial, straightforward refactoring and IMHO we should just push them. Possibly 1-3 should be squashed together, but I posted them separately so they are easier to review. Patches 5 and 6 together implement OClosure. Patch 5 adds the feature and is simple to understand. Patch 6 changes the Closure completion callbacks into OClosure, but because it doesn't

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

Mutable (Int n) => int *n This can currently only be used for callback arguments of type int (not for other types, nor for any ordinary function arguments), but it could be implemented more generally in future. --- generator/generator | 75 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/generator/generator b/generator/generator

As before, but this one has working Python bindings. OCaml still TBD. Rich.

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.

On 2/17/23 17:52, Eric Blake wrote: > On Thu, Feb 16, 2023 at 03:09:02PM +0100, Laszlo Ersek wrote: >> - Py_BuildValue with the "O" format specifier transfers the new list's >> *sole* reference (= ownership) to the just-built higher-level object "args" > > Reference transfer is done with "N", not "O". That would be an >

On Mon, Feb 20, 2023 at 10:45 AM Laszlo Ersek <lersek at redhat.com> wrote: > > On 2/17/23 17:52, Eric Blake wrote: > > On Thu, Feb 16, 2023 at 03:09:02PM +0100, Laszlo Ersek wrote: > > >> - Py_BuildValue with the "O" format specifier transfers the new list's > >> *sole* reference (= ownership) to the just-built higher-level object

This implements most of what I wrote here: https://www.redhat.com/archives/libguestfs/2019-July/msg00213.html

On Wed, Aug 08, 2018 at 06:08:45PM +0300, Nir Soffer wrote: > Can we simplify this these 2 calls? > > nbdkit_error ("%s: %s: error", script, callback); > PyErr_PrintEx (0); No, we want to ensure errors go through the nbdkit_error function so that we can in future implement syslog logging. > In this case we don't to fallback to the simple error print. Do you