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 -=|
diff -r bb529870a27d python/generator.py --- a/python/generator.py Tue Jan 15 12:38:10 2008 -0500 +++ b/python/generator.py Tue Jan 15 18:37:29 2008 -0500 @@ -229,6 +229,7 @@ py_types = { 'double': ('d', None, "double", "double"), 'unsigned int': ('i', None, "int", "int"), 'unsigned long': ('l', None, "long", "long"), + 'unsigned long long': ('l', None, "longlong", "long long"), 'unsigned char *': ('z', None, "charPtr", "char *"), 'char *': ('z', None, "charPtr", "char *"), 'const char *': ('z', None, "charPtrConst", "const char *"), @@ -279,6 +280,11 @@ skip_impl = ( 'virDomainBlockStats', 'virDomainInterfaceStats', 'virNodeGetCellsFreeMemory', + 'virDomainGetSchedulerType', + 'virDomainGetSchedulerParameters', + 'virDomainSetSchedulerParameters', + 'virDomainGetVcpus', + 'virDomainPinVcpu', ) def skip_function(name): @@ -918,6 +924,8 @@ def buildWrappers(): txt.write(" %s()\n" % func); n = 0 for arg in args: + if name == "virDomainPinVcpu" and arg[0] == "maplen": + continue if n != index: classes.write(", %s" % arg[0]) n = n + 1 @@ -939,6 +947,8 @@ def buildWrappers(): classes.write("libvirtmod.%s(" % name) n = 0 for arg in args: + if name == "virDomainPinVcpu" and arg[0] == "maplen": + continue if n != 0: classes.write(", "); if n != index: diff -r bb529870a27d python/libvir.c --- a/python/libvir.c Tue Jan 15 12:38:10 2008 -0500 +++ b/python/libvir.c Tue Jan 15 18:37:29 2008 -0500 @@ -99,6 +99,321 @@ libvirt_virDomainInterfaceStats(PyObject PyTuple_SetItem(info, 7, PyLong_FromLongLong(stats.tx_drop)); return(info); } + + +static PyObject * +libvirt_virDomainGetSchedulerType(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + char *c_retval; + int nparams; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetScedulerType", + &pyobj_domain)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + c_retval = virDomainGetSchedulerType(domain, &nparams); + if (c_retval == NULL) { + Py_INCREF(Py_None); + return(Py_None); + } + + /* convert to a Python tupple of long objects */ + info = PyTuple_New(2); + PyTuple_SetItem(info, 0, libvirt_constcharPtrWrap(c_retval)); + PyTuple_SetItem(info, 1, PyInt_FromLong((long)nparams)); + free(c_retval); + return(info); +} + +static PyObject * +libvirt_virDomainGetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + char *c_retval; + int nparams, i; + virSchedParameterPtr params; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetScedulerParameters", + &pyobj_domain)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + c_retval = virDomainGetSchedulerType(domain, &nparams); + if (c_retval == NULL) { + Py_INCREF(Py_None); + return(Py_None); + } + free(c_retval); + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) { + Py_INCREF(Py_None); + return(Py_None); + } + + if (virDomainGetSchedulerParameters(domain, params, &nparams) < 0) { + free(params); + Py_INCREF(Py_None); + return(Py_None); + } + + /* convert to a Python tupple of long objects */ + info = PyDict_New(); + for (i = 0 ; i < nparams ; i++) { + PyObject *key, *val; + + switch (params[i].type) { + case VIR_DOMAIN_SCHED_FIELD_INT: + val = PyInt_FromLong((long)params[i].value.i); + break; + + case VIR_DOMAIN_SCHED_FIELD_UINT: + val = PyInt_FromLong((long)params[i].value.ui); + break; + + case VIR_DOMAIN_SCHED_FIELD_LLONG: + val = PyLong_FromLongLong((long long)params[i].value.l); + break; + + case VIR_DOMAIN_SCHED_FIELD_ULLONG: + val = PyLong_FromLongLong((long long)params[i].value.ul); + break; + + case VIR_DOMAIN_SCHED_FIELD_DOUBLE: + val = PyFloat_FromDouble((double)params[i].value.d); + break; + + case VIR_DOMAIN_SCHED_FIELD_BOOLEAN: + val = PyBool_FromLong((long)params[i].value.b); + break; + + default: + free(params); + Py_DECREF(info); + Py_INCREF(Py_None); + return (Py_None); + } + + key = libvirt_constcharPtrWrap(params[i].field); + PyDict_SetItem(info, key, val); + } + free(params); + return(info); +} + +static PyObject * +libvirt_virDomainSetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + char *c_retval; + int nparams, i; + virSchedParameterPtr params; + + if (!PyArg_ParseTuple(args, (char *)"OO:virDomainSetScedulerParameters", + &pyobj_domain, &info)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + c_retval = virDomainGetSchedulerType(domain, &nparams); + if (c_retval == NULL) { + Py_INCREF(Py_None); + return(Py_None); + } + free(c_retval); + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) { + Py_INCREF(Py_None); + return(Py_None); + } + + if (virDomainGetSchedulerParameters(domain, params, &nparams) < 0) { + free(params); + Py_INCREF(Py_None); + return(Py_None); + } + + /* convert to a Python tupple of long objects */ + for (i = 0 ; i < nparams ; i++) { + PyObject *key, *val; + key = libvirt_constcharPtrWrap(params[i].field); + val = PyDict_GetItem(info, key); + Py_DECREF(key); + + if (val == NULL) + continue; + + switch (params[i].type) { + case VIR_DOMAIN_SCHED_FIELD_INT: + params[i].value.i = (int)PyInt_AS_LONG(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_UINT: + params[i].value.ui = (unsigned int)PyInt_AS_LONG(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_LLONG: + params[i].value.l = (long long)PyLong_AsLongLong(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_ULLONG: + params[i].value.ul = (unsigned long long)PyLong_AsLongLong(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_DOUBLE: + params[i].value.d = (double)PyFloat_AsDouble(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_BOOLEAN: + { + /* Hack - Python's definition of Py_True breaks strict + * aliasing rules, so can't directly compare :-( + */ + PyObject *hacktrue = PyBool_FromLong(1); + params[i].value.b = hacktrue == val ? 1 : 0; + Py_DECREF(hacktrue); + } + break; + + default: + free(params); + Py_INCREF(Py_None); + return (Py_None); + } + } + + if (virDomainSetSchedulerParameters(domain, params, nparams) < 0) { + free(params); + Py_INCREF(Py_None); + return(Py_None); + } + + free(params); + Py_INCREF(Py_None); + return(Py_None); +} + +static PyObject * +libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *pyretval, *pycpuinfo, *pycpumap; + virNodeInfo nodeinfo; + virDomainInfo dominfo; + virVcpuInfoPtr cpuinfo; + unsigned char *cpumap; + int cpumaplen, i; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetVcpus", + &pyobj_domain)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if (virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo) != 0) { + Py_INCREF(Py_None); + return(Py_None); + } + + if (virDomainGetInfo(domain, &dominfo) != 0) { + Py_INCREF(Py_None); + return(Py_None); + } + + if ((cpuinfo = malloc(sizeof(*cpuinfo)*dominfo.nrVirtCpu)) == NULL) { + Py_INCREF(Py_None); + return(Py_None); + } + cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); + if ((cpumap = malloc(dominfo.nrVirtCpu * cpumaplen)) == NULL) { + free(cpuinfo); + Py_INCREF(Py_None); + return(Py_None); + } + + if (virDomainGetVcpus(domain, + cpuinfo, dominfo.nrVirtCpu, + cpumap, cpumaplen) < 0) { + free(cpuinfo); + free(cpumap); + Py_INCREF(Py_None); + return(Py_None); + } + + /* convert to a Python tupple of long objects */ + pyretval = PyTuple_New(2); + pycpuinfo = PyList_New(dominfo.nrVirtCpu); + pycpumap = PyList_New(dominfo.nrVirtCpu); + + for (i = 0 ; i < dominfo.nrVirtCpu ; i++) { + PyObject *info = PyTuple_New(4); + PyTuple_SetItem(info, 0, PyInt_FromLong((long)cpuinfo[i].number)); + PyTuple_SetItem(info, 1, PyInt_FromLong((long)cpuinfo[i].state)); + PyTuple_SetItem(info, 2, PyLong_FromLongLong((long long)cpuinfo[i].cpuTime)); + PyTuple_SetItem(info, 3, PyInt_FromLong((long)cpuinfo[i].cpu)); + PyList_SetItem(pycpuinfo, i, info); + } + for (i = 0 ; i < dominfo.nrVirtCpu ; i++) { + PyObject *info = PyTuple_New(VIR_NODEINFO_MAXCPUS(nodeinfo)); + int j; + for (j = 0 ; j < VIR_NODEINFO_MAXCPUS(nodeinfo) ; j++) { + PyTuple_SetItem(info, j, PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, i, j))); + } + PyList_SetItem(pycpumap, i, info); + } + PyTuple_SetItem(pyretval, 0, pycpuinfo); + PyTuple_SetItem(pyretval, 1, pycpumap); + + free(cpuinfo); + free(cpumap); + + return(pyretval); +} + + +static PyObject * +libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *pycpumap, *truth; + virNodeInfo nodeinfo; + unsigned char *cpumap; + int cpumaplen, i; + + if (!PyArg_ParseTuple(args, (char *)"OO:virDomainPinVcpu", + &pyobj_domain, &pycpumap)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if (virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo) != 0) { + Py_INCREF(Py_None); + return(Py_None); + } + + cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); + if ((cpumap = malloc(cpumaplen)) == NULL) { + Py_INCREF(Py_None); + return(Py_None); + } + memset(cpumap, 0, cpumaplen); + + truth = PyBool_FromLong(1); + for (i = 0 ; i < VIR_NODEINFO_MAXCPUS(nodeinfo) ; i++) { + PyObject *flag = PyTuple_GetItem(pycpumap, i); + if (flag == truth) + VIR_USE_CPU(cpumap, i); + else + VIR_UNUSE_CPU(cpumap, i); + } + Py_DECREF(truth); + + Py_INCREF(Py_None); + return(Py_None); +} + + /************************************************************************ * * * Global error handler at the Python level * @@ -917,6 +1232,11 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainBlockStats", libvirt_virDomainBlockStats, METH_VARARGS, NULL}, {(char *) "virDomainInterfaceStats", libvirt_virDomainInterfaceStats, METH_VARARGS, NULL}, {(char *) "virNodeGetCellsFreeMemory", libvirt_virNodeGetCellsFreeMemory, METH_VARARGS, NULL}, + {(char *) "virDomainGetSchedulerType", libvirt_virDomainGetSchedulerType, METH_VARARGS, NULL}, + {(char *) "virDomainGetSchedulerParameters", libvirt_virDomainGetSchedulerParameters, METH_VARARGS, NULL}, + {(char *) "virDomainSetSchedulerParameters", libvirt_virDomainSetSchedulerParameters, METH_VARARGS, NULL}, + {(char *) "virDomainGetVcpus", libvirt_virDomainGetVcpus, METH_VARARGS, NULL}, + {(char *) "virDomainPinVcpu", libvirt_virDomainPinVcpu, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} };
-- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list