Eric Blake
2022-Jun-07 02:08 UTC
[Libguestfs] [libnbd PATCH v2 8/8] python: Make nbd.Buffer lighter-weight
Instead of storing a PyCapsule in _o and repeatedly doing lookups to
dereference a stored malloc'd pointer, it is easier to just directly
store a Python buffer-like object as _o. Track initialization via
._init: absent for nbd.Buffer(int), present for
nbd.Buffer.from_bytearray or after we have forced initialization due
to aio_pread or .to_bytearray. At this point, we are still copying in
and out of .{from,to}_bytearray, but we are getting closer to reducing
the number of copies. The sheer size in reduced lines of code is a
testament to the ease of doing things closer to Python, rather than
hidden behind unpacking a PyCapsule layer.
Acceptable side effect: nbd.Buffer(-1) changes from:
RuntimeError: length < 0
to
SystemError: Negative size passed to PyByteArray_FromStringAndSize
The generated code changes as follows:
| --- python/methods.h.bak 2022-06-06 20:36:01.973327739 -0500
| +++ python/methods.h 2022-06-06 20:36:16.146352508 -0500
| @@ -62,9 +62,6 @@
| extern PyObject *nbd_internal_py_close (PyObject *self, PyObject *args);
| extern PyObject *nbd_internal_py_display_version (PyObject *self, PyObject
*args);
| extern PyObject *nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject
*args);
| -extern PyObject *nbd_internal_py_aio_buffer_from_bytearray (PyObject *self,
PyObject *args);
| -extern PyObject *nbd_internal_py_aio_buffer_to_bytearray (PyObject *self,
PyObject *args);
| -extern PyObject *nbd_internal_py_aio_buffer_size (PyObject *self, PyObject
*args);
| extern PyObject *nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject
*args);
| extern PyObject *nbd_internal_py_set_debug (PyObject *self, PyObject *args);
| extern PyObject *nbd_internal_py_get_debug (PyObject *self, PyObject *args);
| --- python/nbd.py.bak 2022-06-06 20:36:01.978327748 -0500
| +++ python/nbd.py 2022-06-06 20:56:41.866729560 -0500
| @@ -134,18 +134,21 @@ class Buffer(object):
| bytearray constructor. Otherwise, ba is copied. Either way, the
| resulting AIO buffer is independent from the original.
| '''
| - o = libnbdmod.aio_buffer_from_bytearray(ba)
| self = cls(0)
| - self._o = o
| + self._o = bytearray(ba)
| + self._init = True
| return self
|
| def to_bytearray(self):
| '''Copy an AIO buffer into a bytearray.'''
| - return libnbdmod.aio_buffer_to_bytearray(self)
| + if not hasattr(self, '_init'):
| + self._o = bytearray(len(self._o))
| + self._init = True
| + return bytearray(self._o)
|
| def size(self):
| '''Return the size of an AIO buffer.'''
| - return libnbdmod.aio_buffer_size(self)
| + return len(self._o)
|
| def is_zero(self, offset=0, size=-1):
| '''Returns true if and only if all bytes in the buffer
are zeroes.
| @@ -161,7 +164,8 @@ class Buffer(object):
| always returns true. If size > 0, we check the interval
| [offset..offset+size-1].
| '''
| - return libnbdmod.aio_buffer_is_zero(self, offset, size)
| + return libnbdmod.aio_buffer_is_zero(self._o, offset, size,
| + hasattr(self, '_init'))
|
|
| class NBD(object):
---
generator/Python.ml | 20 ++--
python/handle.c | 242 +++++++++-----------------------------------
2 files changed, 56 insertions(+), 206 deletions(-)
diff --git a/generator/Python.ml b/generator/Python.ml
index c49af4f..86781ac 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -73,9 +73,6 @@ let
) ([ "create"; "close";
"display_version";
"alloc_aio_buffer";
- "aio_buffer_from_bytearray";
- "aio_buffer_to_bytearray";
- "aio_buffer_size";
"aio_buffer_is_zero"] @ List.map fst handle_calls);
pr "\n";
@@ -105,9 +102,6 @@ let
) ([ "create"; "close";
"display_version";
"alloc_aio_buffer";
- "aio_buffer_from_bytearray";
- "aio_buffer_to_bytearray";
- "aio_buffer_size";
"aio_buffer_is_zero"] @ List.map fst handle_calls);
pr " { NULL, NULL, 0, NULL }\n";
pr "};\n";
@@ -748,18 +742,21 @@ let
bytearray constructor. Otherwise, ba is copied. Either way, the
resulting AIO buffer is independent from the original.
'''
- o = libnbdmod.aio_buffer_from_bytearray(ba)
self = cls(0)
- self._o = o
+ self._o = bytearray(ba)
+ self._init = True
return self
def to_bytearray(self):
'''Copy an AIO buffer into a bytearray.'''
- return libnbdmod.aio_buffer_to_bytearray(self)
+ if not hasattr(self, '_init'):
+ self._o = bytearray(len(self._o))
+ self._init = True
+ return bytearray(self._o)
def size(self):
'''Return the size of an AIO buffer.'''
- return libnbdmod.aio_buffer_size(self)
+ return len(self._o)
def is_zero(self, offset=0, size=-1):
'''Returns true if and only if all bytes in the buffer are
zeroes.
@@ -775,7 +772,8 @@ let
always returns true. If size > 0, we check the interval
[offset..offset+size-1].
'''
- return libnbdmod.aio_buffer_is_zero(self, offset, size)
+ return libnbdmod.aio_buffer_is_zero(self._o, offset, size,
+ hasattr(self, '_init'))
class NBD(object):
diff --git a/python/handle.c b/python/handle.c
index 79b369d..ca6c8e9 100644
--- a/python/handle.c
+++ b/python/handle.c
@@ -40,12 +40,6 @@
#include "methods.h"
-struct py_aio_buffer {
- Py_ssize_t len;
- void *data;
- bool initialized;
-};
-
static inline PyObject *
put_handle (struct nbd_handle *h)
{
@@ -102,242 +96,100 @@ nbd_internal_py_display_version (PyObject *self, PyObject
*args)
return Py_None;
}
-static const char aio_buffer_name[] = "nbd.Buffer";
-
-/* Return internal buffer pointer of nbd.Buffer */
-static struct py_aio_buffer *
-nbd_internal_py_get_aio_buffer (PyObject *object)
+/* Return new reference to MemoryView wrapping aio_buffer contents */
+PyObject *
+nbd_internal_py_get_aio_view (PyObject *object, bool require_init)
{
+ PyObject *buffer = NULL;
+
if (PyObject_IsInstance (object, nbd_internal_py_get_nbd_buffer_type ())) {
- PyObject *capsule = PyObject_GetAttrString(object, "_o");
+ buffer = PyObject_GetAttrString (object, "_o");
- return PyCapsule_GetPointer (capsule, aio_buffer_name);
+ if (require_init && ! PyObject_HasAttrString (object,
"_init")) {
+ assert (PyByteArray_Check (buffer));
+ memset (PyByteArray_AS_STRING (buffer), 0,
+ PyByteArray_GET_SIZE (buffer));
+ if (PyObject_SetAttrString (object, "_init", Py_True) < 0)
+ return NULL;
+ }
}
+ if (buffer)
+ return PyMemoryView_FromObject (buffer);
+
PyErr_SetString (PyExc_TypeError,
"aio_buffer: expecting nbd.Buffer instance");
return NULL;
}
-/* Return new reference to MemoryView wrapping aio_buffer contents */
-PyObject *
-nbd_internal_py_get_aio_view (PyObject *object, bool require_init)
-{
- struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object);
-
- if (!buf)
- return NULL;
-
- if (require_init && !buf->initialized) {
- memset (buf->data, 0, buf->len);
- buf->initialized = true;
- }
-
- return PyMemoryView_FromMemory (buf->data, buf->len,
- require_init ? PyBUF_READ : PyBUF_WRITE);
-}
-
int
nbd_internal_py_init_aio_buffer (PyObject *object)
{
- struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object);
-
- assert (buf);
- buf->initialized = true;
+ if (PyObject_IsInstance (object, nbd_internal_py_get_nbd_buffer_type ()))
+ return PyObject_SetAttrString (object, "_init", Py_True);
return 0;
}
-static void
-free_aio_buffer (PyObject *capsule)
-{
- struct py_aio_buffer *buf = PyCapsule_GetPointer (capsule, aio_buffer_name);
-
- if (buf)
- free (buf->data);
- free (buf);
-}
-
-/* Allocate a persistent buffer used for nbd_aio_pread. */
+/* Allocate an uninitialized persistent buffer used for nbd_aio_pread. */
PyObject *
nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args)
{
- struct py_aio_buffer *buf;
- PyObject *ret;
+ Py_ssize_t len;
- buf = malloc (sizeof *buf);
- if (buf == NULL) {
- PyErr_NoMemory ();
- return NULL;
- }
-
- buf->initialized = false;
if (!PyArg_ParseTuple (args, "n:nbd_internal_py_alloc_aio_buffer",
- &buf->len)) {
- free (buf);
+ &len))
return NULL;
- }
- if (buf->len < 0) {
- PyErr_SetString (PyExc_RuntimeError, "length < 0");
- free (buf);
- return NULL;
- }
- buf->data = malloc (buf->len);
- if (buf->data == NULL) {
- PyErr_NoMemory ();
- free (buf);
- return NULL;
- }
-
- ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
- if (ret == NULL) {
- free (buf->data);
- free (buf);
- return NULL;
- }
-
- return ret;
-}
-
-PyObject *
-nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args)
-{
- PyObject *obj;
- PyObject *arr = NULL;
- Py_ssize_t len;
- void *data;
- struct py_aio_buffer *buf;
- PyObject *ret;
-
- if (!PyArg_ParseTuple (args,
-
"O:nbd_internal_py_aio_buffer_from_bytearray",
- &obj))
- return NULL;
-
- if (! PyByteArray_Check (obj)) {
- arr = PyByteArray_FromObject (obj);
- if (arr == NULL)
- return NULL;
- obj = arr;
- }
- data = PyByteArray_AsString (obj);
- if (!data) {
- PyErr_SetString (PyExc_RuntimeError,
- "parameter is not a bytearray or buffer");
- Py_XDECREF (arr);
- return NULL;
- }
- len = PyByteArray_Size (obj);
-
- buf = malloc (sizeof *buf);
- if (buf == NULL) {
- PyErr_NoMemory ();
- Py_XDECREF (arr);
- return NULL;
- }
-
- buf->len = len;
- buf->data = malloc (len);
- if (buf->data == NULL) {
- PyErr_NoMemory ();
- free (buf);
- Py_XDECREF (arr);
- return NULL;
- }
- memcpy (buf->data, data, len);
- Py_XDECREF (arr);
- buf->initialized = true;
-
- ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
- if (ret == NULL) {
- free (buf->data);
- free (buf);
- return NULL;
- }
-
- return ret;
-}
-
-PyObject *
-nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args)
-{
- PyObject *obj;
- PyObject *view;
- PyObject *ret;
-
- if (!PyArg_ParseTuple (args,
- "O:nbd_internal_py_aio_buffer_to_bytearray",
- &obj))
- return NULL;
-
- view = nbd_internal_py_get_aio_view (obj, true);
- if (view == NULL)
- return NULL;
-
- ret = PyByteArray_FromObject (view);
- Py_DECREF (view);
- return ret;
-}
-
-PyObject *
-nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args)
-{
- PyObject *obj;
- struct py_aio_buffer *buf;
-
- if (!PyArg_ParseTuple (args,
- "O:nbd_internal_py_aio_buffer_size",
- &obj))
- return NULL;
-
- buf = nbd_internal_py_get_aio_buffer (obj);
- if (buf == NULL)
- return NULL;
-
- return PyLong_FromSsize_t (buf->len);
+ /* Constructing bytearray(len) in python zeroes the memory; doing it this
+ * way gives uninitialized memory. This correctly flags negative len.
+ */
+ return PyByteArray_FromStringAndSize (NULL, len);
}
PyObject *
nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args)
{
- PyObject *obj;
- struct py_aio_buffer *buf;
+ Py_buffer buf;
Py_ssize_t offset, size;
+ int init;
+ PyObject *ret = NULL;
if (!PyArg_ParseTuple (args,
- "Onn:nbd_internal_py_aio_buffer_is_zero",
- &obj, &offset, &size))
+ "y*nnp:nbd_internal_py_aio_buffer_is_zero",
+ &buf, &offset, &size, &init))
return NULL;
- if (size == 0)
- Py_RETURN_TRUE;
-
- buf = nbd_internal_py_get_aio_buffer (obj);
- if (buf == NULL)
- return NULL;
+ if (size == 0) {
+ ret = Py_True;
+ goto out;
+ }
/* Check the bounds of the offset. */
- if (offset < 0 || offset > buf->len) {
+ if (offset < 0 || offset > buf.len) {
PyErr_SetString (PyExc_IndexError, "offset out of range");
- return NULL;
+ goto out;
}
/* Compute or check the length. */
if (size == -1)
- size = buf->len - offset;
+ size = buf.len - offset;
else if (size < 0) {
PyErr_SetString (PyExc_IndexError,
"size cannot be negative, "
"except -1 to mean to the end of the buffer");
- return NULL;
+ goto out;
}
- else if ((size_t) offset + size > buf->len) {
+ else if ((size_t) offset + size > buf.len) {
PyErr_SetString (PyExc_IndexError, "size out of range");
- return NULL;
+ goto out;
}
- if (!buf->initialized || is_zero (buf->data + offset, size))
- Py_RETURN_TRUE;
+ if (!init || is_zero (buf.buf + offset, size))
+ ret = Py_True;
else
- Py_RETURN_FALSE;
+ ret = Py_False;
+ out:
+ PyBuffer_Release (&buf);
+ Py_XINCREF (ret);
+ return ret;
}
--
2.36.1
Richard W.M. Jones
2022-Jun-07 13:20 UTC
[Libguestfs] [libnbd PATCH v2 8/8] python: Make nbd.Buffer lighter-weight
On Mon, Jun 06, 2022 at 09:08:33PM -0500, Eric Blake wrote:> Instead of storing a PyCapsule in _o and repeatedly doing lookups to > dereference a stored malloc'd pointer, it is easier to just directly > store a Python buffer-like object as _o. Track initialization via > ._init: absent for nbd.Buffer(int), present for > nbd.Buffer.from_bytearray or after we have forced initialization due > to aio_pread or .to_bytearray. At this point, we are still copying in > and out of .{from,to}_bytearray, but we are getting closer to reducing > the number of copies. The sheer size in reduced lines of code is a > testament to the ease of doing things closer to Python, rather than > hidden behind unpacking a PyCapsule layer. > > Acceptable side effect: nbd.Buffer(-1) changes from: > RuntimeError: length < 0 > to > SystemError: Negative size passed to PyByteArray_FromStringAndSize > > The generated code changes as follows: > > | --- python/methods.h.bak 2022-06-06 20:36:01.973327739 -0500 > | +++ python/methods.h 2022-06-06 20:36:16.146352508 -0500 > | @@ -62,9 +62,6 @@ > | extern PyObject *nbd_internal_py_close (PyObject *self, PyObject *args); > | extern PyObject *nbd_internal_py_display_version (PyObject *self, PyObject *args); > | extern PyObject *nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args); > | -extern PyObject *nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args); > | -extern PyObject *nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args); > | -extern PyObject *nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args); > | extern PyObject *nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args); > | extern PyObject *nbd_internal_py_set_debug (PyObject *self, PyObject *args); > | extern PyObject *nbd_internal_py_get_debug (PyObject *self, PyObject *args); > | --- python/nbd.py.bak 2022-06-06 20:36:01.978327748 -0500 > | +++ python/nbd.py 2022-06-06 20:56:41.866729560 -0500 > | @@ -134,18 +134,21 @@ class Buffer(object): > | bytearray constructor. Otherwise, ba is copied. Either way, the > | resulting AIO buffer is independent from the original. > | ''' > | - o = libnbdmod.aio_buffer_from_bytearray(ba) > | self = cls(0) > | - self._o = o > | + self._o = bytearray(ba) > | + self._init = True > | return self > | > | def to_bytearray(self): > | '''Copy an AIO buffer into a bytearray.''' > | - return libnbdmod.aio_buffer_to_bytearray(self) > | + if not hasattr(self, '_init'): > | + self._o = bytearray(len(self._o)) > | + self._init = True > | + return bytearray(self._o) > | > | def size(self): > | '''Return the size of an AIO buffer.''' > | - return libnbdmod.aio_buffer_size(self) > | + return len(self._o) > | > | def is_zero(self, offset=0, size=-1): > | '''Returns true if and only if all bytes in the buffer are zeroes. > | @@ -161,7 +164,8 @@ class Buffer(object): > | always returns true. If size > 0, we check the interval > | [offset..offset+size-1]. > | ''' > | - return libnbdmod.aio_buffer_is_zero(self, offset, size) > | + return libnbdmod.aio_buffer_is_zero(self._o, offset, size, > | + hasattr(self, '_init')) > | > | > | class NBD(object): > --- > generator/Python.ml | 20 ++-- > python/handle.c | 242 +++++++++----------------------------------- > 2 files changed, 56 insertions(+), 206 deletions(-) > > diff --git a/generator/Python.ml b/generator/Python.ml > index c49af4f..86781ac 100644 > --- a/generator/Python.ml > +++ b/generator/Python.ml > @@ -73,9 +73,6 @@ let > ) ([ "create"; "close"; > "display_version"; > "alloc_aio_buffer"; > - "aio_buffer_from_bytearray"; > - "aio_buffer_to_bytearray"; > - "aio_buffer_size"; > "aio_buffer_is_zero"] @ List.map fst handle_calls); > > pr "\n"; > @@ -105,9 +102,6 @@ let > ) ([ "create"; "close"; > "display_version"; > "alloc_aio_buffer"; > - "aio_buffer_from_bytearray"; > - "aio_buffer_to_bytearray"; > - "aio_buffer_size"; > "aio_buffer_is_zero"] @ List.map fst handle_calls); > pr " { NULL, NULL, 0, NULL }\n"; > pr "};\n"; > @@ -748,18 +742,21 @@ let > bytearray constructor. Otherwise, ba is copied. Either way, the > resulting AIO buffer is independent from the original. > ''' > - o = libnbdmod.aio_buffer_from_bytearray(ba) > self = cls(0) > - self._o = o > + self._o = bytearray(ba) > + self._init = True > return self > > def to_bytearray(self): > '''Copy an AIO buffer into a bytearray.''' > - return libnbdmod.aio_buffer_to_bytearray(self) > + if not hasattr(self, '_init'): > + self._o = bytearray(len(self._o)) > + self._init = True > + return bytearray(self._o) > > def size(self): > '''Return the size of an AIO buffer.''' > - return libnbdmod.aio_buffer_size(self) > + return len(self._o) > > def is_zero(self, offset=0, size=-1): > '''Returns true if and only if all bytes in the buffer are zeroes. > @@ -775,7 +772,8 @@ let > always returns true. If size > 0, we check the interval > [offset..offset+size-1]. > ''' > - return libnbdmod.aio_buffer_is_zero(self, offset, size) > + return libnbdmod.aio_buffer_is_zero(self._o, offset, size, > + hasattr(self, '_init')) > > > class NBD(object): > diff --git a/python/handle.c b/python/handle.c > index 79b369d..ca6c8e9 100644 > --- a/python/handle.c > +++ b/python/handle.c > @@ -40,12 +40,6 @@ > > #include "methods.h" > > -struct py_aio_buffer { > - Py_ssize_t len; > - void *data; > - bool initialized; > -}; > - > static inline PyObject * > put_handle (struct nbd_handle *h) > { > @@ -102,242 +96,100 @@ nbd_internal_py_display_version (PyObject *self, PyObject *args) > return Py_None; > } > > -static const char aio_buffer_name[] = "nbd.Buffer"; > - > -/* Return internal buffer pointer of nbd.Buffer */ > -static struct py_aio_buffer * > -nbd_internal_py_get_aio_buffer (PyObject *object) > +/* Return new reference to MemoryView wrapping aio_buffer contents */ > +PyObject * > +nbd_internal_py_get_aio_view (PyObject *object, bool require_init) > { > + PyObject *buffer = NULL; > + > if (PyObject_IsInstance (object, nbd_internal_py_get_nbd_buffer_type ())) { > - PyObject *capsule = PyObject_GetAttrString(object, "_o"); > + buffer = PyObject_GetAttrString (object, "_o"); > > - return PyCapsule_GetPointer (capsule, aio_buffer_name); > + if (require_init && ! PyObject_HasAttrString (object, "_init")) { > + assert (PyByteArray_Check (buffer)); > + memset (PyByteArray_AS_STRING (buffer), 0, > + PyByteArray_GET_SIZE (buffer)); > + if (PyObject_SetAttrString (object, "_init", Py_True) < 0) > + return NULL; > + } > } > > + if (buffer) > + return PyMemoryView_FromObject (buffer); > + > PyErr_SetString (PyExc_TypeError, > "aio_buffer: expecting nbd.Buffer instance"); > return NULL; > } > > -/* Return new reference to MemoryView wrapping aio_buffer contents */ > -PyObject * > -nbd_internal_py_get_aio_view (PyObject *object, bool require_init) > -{ > - struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object); > - > - if (!buf) > - return NULL; > - > - if (require_init && !buf->initialized) { > - memset (buf->data, 0, buf->len); > - buf->initialized = true; > - } > - > - return PyMemoryView_FromMemory (buf->data, buf->len, > - require_init ? PyBUF_READ : PyBUF_WRITE); > -} > - > int > nbd_internal_py_init_aio_buffer (PyObject *object) > { > - struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object); > - > - assert (buf); > - buf->initialized = true; > + if (PyObject_IsInstance (object, nbd_internal_py_get_nbd_buffer_type ())) > + return PyObject_SetAttrString (object, "_init", Py_True); > return 0; > } > > -static void > -free_aio_buffer (PyObject *capsule) > -{ > - struct py_aio_buffer *buf = PyCapsule_GetPointer (capsule, aio_buffer_name); > - > - if (buf) > - free (buf->data); > - free (buf); > -} > - > -/* Allocate a persistent buffer used for nbd_aio_pread. */ > +/* Allocate an uninitialized persistent buffer used for nbd_aio_pread. */ > PyObject * > nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args) > { > - struct py_aio_buffer *buf; > - PyObject *ret; > + Py_ssize_t len; > > - buf = malloc (sizeof *buf); > - if (buf == NULL) { > - PyErr_NoMemory (); > - return NULL; > - } > - > - buf->initialized = false; > if (!PyArg_ParseTuple (args, "n:nbd_internal_py_alloc_aio_buffer", > - &buf->len)) { > - free (buf); > + &len)) > return NULL; > - } > > - if (buf->len < 0) { > - PyErr_SetString (PyExc_RuntimeError, "length < 0"); > - free (buf); > - return NULL; > - } > - buf->data = malloc (buf->len); > - if (buf->data == NULL) { > - PyErr_NoMemory (); > - free (buf); > - return NULL; > - } > - > - ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer); > - if (ret == NULL) { > - free (buf->data); > - free (buf); > - return NULL; > - } > - > - return ret; > -} > - > -PyObject * > -nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args) > -{ > - PyObject *obj; > - PyObject *arr = NULL; > - Py_ssize_t len; > - void *data; > - struct py_aio_buffer *buf; > - PyObject *ret; > - > - if (!PyArg_ParseTuple (args, > - "O:nbd_internal_py_aio_buffer_from_bytearray", > - &obj)) > - return NULL; > - > - if (! PyByteArray_Check (obj)) { > - arr = PyByteArray_FromObject (obj); > - if (arr == NULL) > - return NULL; > - obj = arr; > - } > - data = PyByteArray_AsString (obj); > - if (!data) { > - PyErr_SetString (PyExc_RuntimeError, > - "parameter is not a bytearray or buffer"); > - Py_XDECREF (arr); > - return NULL; > - } > - len = PyByteArray_Size (obj); > - > - buf = malloc (sizeof *buf); > - if (buf == NULL) { > - PyErr_NoMemory (); > - Py_XDECREF (arr); > - return NULL; > - } > - > - buf->len = len; > - buf->data = malloc (len); > - if (buf->data == NULL) { > - PyErr_NoMemory (); > - free (buf); > - Py_XDECREF (arr); > - return NULL; > - } > - memcpy (buf->data, data, len); > - Py_XDECREF (arr); > - buf->initialized = true; > - > - ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer); > - if (ret == NULL) { > - free (buf->data); > - free (buf); > - return NULL; > - } > - > - return ret; > -} > - > -PyObject * > -nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args) > -{ > - PyObject *obj; > - PyObject *view; > - PyObject *ret; > - > - if (!PyArg_ParseTuple (args, > - "O:nbd_internal_py_aio_buffer_to_bytearray", > - &obj)) > - return NULL; > - > - view = nbd_internal_py_get_aio_view (obj, true); > - if (view == NULL) > - return NULL; > - > - ret = PyByteArray_FromObject (view); > - Py_DECREF (view); > - return ret; > -} > - > -PyObject * > -nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args) > -{ > - PyObject *obj; > - struct py_aio_buffer *buf; > - > - if (!PyArg_ParseTuple (args, > - "O:nbd_internal_py_aio_buffer_size", > - &obj)) > - return NULL; > - > - buf = nbd_internal_py_get_aio_buffer (obj); > - if (buf == NULL) > - return NULL; > - > - return PyLong_FromSsize_t (buf->len); > + /* Constructing bytearray(len) in python zeroes the memory; doing it this > + * way gives uninitialized memory. This correctly flags negative len. > + */ > + return PyByteArray_FromStringAndSize (NULL, len); > } > > PyObject * > nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args) > { > - PyObject *obj; > - struct py_aio_buffer *buf; > + Py_buffer buf; > Py_ssize_t offset, size; > + int init; > + PyObject *ret = NULL; > > if (!PyArg_ParseTuple (args, > - "Onn:nbd_internal_py_aio_buffer_is_zero", > - &obj, &offset, &size)) > + "y*nnp:nbd_internal_py_aio_buffer_is_zero", > + &buf, &offset, &size, &init)) > return NULL; > > - if (size == 0) > - Py_RETURN_TRUE; > - > - buf = nbd_internal_py_get_aio_buffer (obj); > - if (buf == NULL) > - return NULL; > + if (size == 0) { > + ret = Py_True; > + goto out; > + } > > /* Check the bounds of the offset. */ > - if (offset < 0 || offset > buf->len) { > + if (offset < 0 || offset > buf.len) { > PyErr_SetString (PyExc_IndexError, "offset out of range"); > - return NULL; > + goto out; > } > > /* Compute or check the length. */ > if (size == -1) > - size = buf->len - offset; > + size = buf.len - offset; > else if (size < 0) { > PyErr_SetString (PyExc_IndexError, > "size cannot be negative, " > "except -1 to mean to the end of the buffer"); > - return NULL; > + goto out; > } > - else if ((size_t) offset + size > buf->len) { > + else if ((size_t) offset + size > buf.len) { > PyErr_SetString (PyExc_IndexError, "size out of range"); > - return NULL; > + goto out; > } > > - if (!buf->initialized || is_zero (buf->data + offset, size)) > - Py_RETURN_TRUE; > + if (!init || is_zero (buf.buf + offset, size)) > + ret = Py_True; > else > - Py_RETURN_FALSE; > + ret = Py_False; > + out: > + PyBuffer_Release (&buf); > + Py_XINCREF (ret); > + return ret; > }Looks like a nice simplification! Reviewed-by: Richard W.M. Jones <rjones at redhat.com> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v