Re: [PATCH] Return right error code for baselineCPU

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

 



On 12/14/2013 01:36 PM, Daniel P. Berrange wrote:
> On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote:
>> On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger <n0ano@xxxxxxxxx> wrote:
>>>
>>> This Python interface code is returning a -1 on errors for the
>>> `baselineCPU' API.  Since this API is supposed to return a pointer
>>> the error return value should really be VIR_PY_NONE.
>>>
>>> NB:  I've checked all the other APIs in this file and this is the
>>> only pointer API that is returning -1.
>>>
>>> Signed-off-by: Don Dugger <donald.d.dugger@xxxxxxxxx>
>>> ---
>>>  python/libvirt-override.c |    8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/python/libvirt-override.c b/python/libvirt-override.c
>>> index c60747d..b471605 100644
>>> --- a/python/libvirt-override.c
>>> +++ b/python/libvirt-override.c
>>> @@ -4471,13 +4471,13 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED,
>>>
>>>          ncpus = PyList_Size(list);
>>>          if (VIR_ALLOC_N_QUIET(xmlcpus, ncpus) < 0)
>>> -            return VIR_PY_INT_FAIL;
>>> +            return VIR_PY_NONE;
>>>
>>>          for (i = 0; i < ncpus; i++) {
>>>              xmlcpus[i] = PyString_AsString(PyList_GetItem(list, i));
>>>              if (xmlcpus[i] == NULL) {
>>>                  VIR_FREE(xmlcpus);
>>> -                return VIR_PY_INT_FAIL;
>>> +                return VIR_PY_NONE;
>>>              }
>>>          }
>>>      }
>>> @@ -4489,13 +4489,13 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED,
>>>      VIR_FREE(xmlcpus);
>>>
>>>      if (base_cpu == NULL)
>>> -        return VIR_PY_INT_FAIL;
>>> +        return VIR_PY_NONE;
>>>
>>>      pybase_cpu = PyString_FromString(base_cpu);
>>>      VIR_FREE(base_cpu);
>>>
>>>      if (pybase_cpu == NULL)
>>> -        return VIR_PY_INT_FAIL;
>>> +        return VIR_PY_NONE;
>>>
>>>      return pybase_cpu;
>>>  }
>>> --
>>> 1.7.10.4
>>>
>>
>> ACK. This is correct. But it obviously changes our API so I'm not
>> really sure how we should handle this, (e.g. document the API as is as
>> note that its broken or fix it).
> 
> The implicit expectation with python APIs is that they all raise an
> exception if the libvirt call fails. So ACK to this bug fix & we
> should put it in maint branches.
> 
> That said, please hold off applying this to GIT. We'll put it into
> the new libvirt-python git I'm about to push instead.
> 

I've pushed this to 0.10.2 and 1.0.5 maint branches now.

Thanks,
Cole

--
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]