On 11/26/2013 11:32 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> The virNodeGetSecurityModel, virDomainGetSecurityLabel and
> virDomainGetSecurityLabelList methods were disabled in the
> python binding for inexplicable reasons.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> +++ b/libvirt-override.c
> @@ -2865,6 +2865,83 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
> }
>
> static PyObject *
> +libvirt_virNodeGetSecurityModel(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
{ on its own line (Hmm, we aren't very consistent in this file)
> + PyObject *py_retval;
> + int c_retval;
> + virConnectPtr conn;
> + PyObject *pyobj_conn;
> + virSecurityModel model;
> +
> + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetSecurityModel", &pyobj_conn))
> + return NULL;
> + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
> +
> + LIBVIRT_BEGIN_ALLOW_THREADS;
> + c_retval = virNodeGetSecurityModel(conn, &model);
> + LIBVIRT_END_ALLOW_THREADS;
> + if (c_retval < 0)
> + return VIR_PY_NONE;
> + py_retval = PyList_New(2);
If PyList_New() fails, py_retval is NULL,...
> + PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(&model.model[0]));
...and this will crash. Instead, you should just return NULL early.
(Then again, we have LOTS of places where we aren't checking the return
of PyList_SetItem for failure, which is a major cleanup in its own right).
> + PyList_SetItem(py_retval, 1, libvirt_constcharPtrWrap(&model.doi[0]));
> + return py_retval;
> +}
> +
> +static PyObject *
> +libvirt_virDomainGetSecurityLabel(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
{ placement
> + py_retval = PyList_New(2);
> + PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(&label.label[0]));
Same comment on error handling.
> + PyList_SetItem(py_retval, 1, libvirt_boolWrap(label.enforcing));
> + return py_retval;
> +}
> +
> +#if LIBVIR_CHECK_VERSION(0, 9, 13)
> +static PyObject *
> +libvirt_virDomainGetSecurityLabelList(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
{ placement
> + py_retval = PyList_New(0);
> + for (i = 0 ; i < c_retval ; i++) {
No space before semicolon (hmm, we lost our syntax checker in the split)
> + PyObject *entry = PyList_New(2);
> + PyList_SetItem(entry, 0, libvirt_constcharPtrWrap(&labels[i].label[0]));
Again, can't libvirt_constcharPtrWrap() return NULL (on OOM situations),
which will crash PyList_SetItem()?
Only findings are style-related or part of a bigger cleanup needed for
proper error handling, so ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list