[Libguestfs] [PATCH libnbd v2 2/3] python: Add nbd.Buffer free() method.

Richard W.M. Jones rjones at redhat.com
Sun Aug 11 10:26:12 UTC 2019


As in OCaml we need to manually free the buffer, so this commit adds a
free() method to the buffer.

Note (as in OCaml) you must keep a reference to the persistent buffer.
The following code will trivially segfault:

  buf = nbd.Buffer (512)
  nbd.aio_pread (buf, 0)
  # allow buf to go out of scope here

Writing correct code is therefore difficult, but doing it properly
will require cooperation from libnbd so we can increment the refcount
when aio_pread() is called and decrement the refcount when libnbd no
longer holds a reference to the buffer internally.
---
 generator/generator | 14 +++++++++++++-
 python/handle.c     | 23 +++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/generator/generator b/generator/generator
index e5d9aaa..f6c1454 100755
--- a/generator/generator
+++ b/generator/generator
@@ -4081,6 +4081,7 @@ raise_exception ()
          name;
   ) ([ "create"; "close";
        "alloc_aio_buffer";
+       "free_aio_buffer";
        "aio_buffer_from_bytearray";
        "aio_buffer_to_bytearray";
        "aio_buffer_size" ] @ List.map fst handle_calls);
@@ -4111,6 +4112,7 @@ let generate_python_libnbdmod_c () =
          name name;
   ) ([ "create"; "close";
        "alloc_aio_buffer";
+       "free_aio_buffer";
        "aio_buffer_from_bytearray";
        "aio_buffer_to_bytearray";
        "aio_buffer_size" ] @ List.map fst handle_calls);
@@ -4394,7 +4396,12 @@ let print_python_binding name { args; optargs; ret; may_set_error } =
     | BytesOut (n, count) ->
        pr "  %s = malloc (%s);\n" n count
     | BytesPersistIn (n, _) | BytesPersistOut (n, _) ->
-       pr "  %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n
+       pr "  %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n;
+       pr "  if (%s_buf->data == NULL) {\n" n;
+       pr "    PyErr_SetString (PyExc_RuntimeError,\n";
+       pr "                     \"nbd.Buffer has been freed\");\n";
+       pr "    return NULL;\n";
+       pr "  }\n";
     | Closure { cbname } ->
        pr "  /* Increment refcount since pointer may be saved by libnbd. */\n";
        pr "  Py_INCREF (%s_user_data);\n" cbname;
@@ -4651,6 +4658,11 @@ class Buffer (object):
         '''allocate an uninitialized AIO buffer used for nbd.aio_pread'''
         self._o = libnbdmod.alloc_aio_buffer (len)
 
+    def free (self):
+        '''free the buffer:
+        you should call this in the completion callback'''
+        libnbdmod.free_aio_buffer (self._o)
+
     @classmethod
     def from_bytearray (cls, ba):
         '''create an AIO buffer from a bytearray'''
diff --git a/python/handle.c b/python/handle.c
index 4b510ad..2de8712 100644
--- a/python/handle.c
+++ b/python/handle.c
@@ -89,6 +89,29 @@ free_aio_buffer (PyObject *capsule)
   free (buf);
 }
 
+PyObject *
+nbd_internal_py_free_aio_buffer (PyObject *self, PyObject *args)
+{
+  PyObject *obj;
+  struct py_aio_buffer *buf;
+
+  if (!PyArg_ParseTuple (args,
+                         (char *) "O:nbd_internal_py_free_aio_buffer",
+                         &obj))
+    return NULL;
+
+  buf = nbd_internal_py_get_aio_buffer (obj);
+  if (buf == NULL)
+    return NULL;
+
+  free (buf->data);
+  /* Set this to NULL to avoid double-free in the destructor. */
+  buf->data = NULL;
+
+  Py_INCREF (Py_None);
+  return Py_None;
+}
+
 /* Allocate a persistent buffer used for nbd_aio_pread. */
 PyObject *
 nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args)
-- 
2.22.0




More information about the Libguestfs mailing list