[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] handle PyTuple_New's malloc failure



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}
 };
 

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]