Re: [PATCH python] Add missing binding of security model/label APIs

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

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]