On Thu, Jan 17, 2008 at 04:41:49PM +0100, Jim Meyering wrote:
python/libvir.c calls PyTuple_New, but doesn't handle the
possibility
of a NULL return value. Subsequent use of the result pointer in e.g.,
PyTuple_SetItem, would dereference it.
This fixes most of them, but leaves the ones in
virConnectCredCallbackWrapper untouched. Fixing them
properly is not as easy, and IMHO will require actual testing.
In this patch, I combined each new test like this
((info = PyTuple_New(8)) == NULL)
with the preceding "if (c_retval < 0)", since the bodies would be the same.
Duplicating that 2-line body might look more readable,
depending on which side of the bed you get up on...
I can go either way.
I think it'd be nicer to keep the PyTuple_new bit separate as it is
now, because in some APIs there can be other code between the existing
c_retval < 0 check, and the point at which we create the Tuple.
eg, the libvirt_virDomainGetSchedulerParameters method in the
patch I'm attaching which implements a number of missing APIs
NB, this patch is not tested yet, so not intended to be applied
Might be useful to have a macro to combine the Py_INCREF and return
(Py_None) in one convenient blob. return VIR_PY_NONE();
Subject: [PATCH] python/libvir.c: Handle PyTuple_New's malloc
failure.
---
python/libvir.c | 27 ++++++++++++---------------
1 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/python/libvir.c b/python/libvir.c
index 3b41dc1..13b809f 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -48,17 +48,16 @@ libvirt_virDomainBlockStats(PyObject *self ATTRIBUTE_UNUSED, PyObject
*args) {
if (!PyArg_ParseTuple(args, (char *)"Oz:virDomainBlockStats",
&pyobj_domain,&path))
- return(NULL);
+ return(NULL);
I curious as to why we ever return(NULL) here rather than Py_None. I'm
not sure of the python runtime / C binding contract - but returning an
actual NULL seems odd.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules:
http://search.cpan.org/~danberr/ -=|
|=- Projects:
http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|