Re: [Qemu-devel] [PATCH v2 1/4] Add Error **errp for xen_host_pci_device_get()

[Cao jin <caoj.fnst@cn.fujitsu.com>]

On 01/04/2016 11:15 PM, Stefano Stabellini wrote:
> On Sun, 27 Dec 2015, Cao jin wrote:
>> To catch the error msg. Also modify the caller
>>
>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>
> This looks much better, thanks.
>
>
[...]
>>
>> -int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
>> -                            uint8_t bus, uint8_t dev, uint8_t func)
>> +void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
>> +                            uint8_t bus, uint8_t dev, uint8_t func,
>> +                            Error **errp)
>>   {
>>       unsigned int v;
>> -    int rc = 0;
>>
>>       d->config_fd = -1;
>>       d->domain = domain;
>> @@ -353,43 +360,48 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
>>       d->dev = dev;
>>       d->func = func;
>>
>> -    rc = xen_host_pci_config_open(d);
>> -    if (rc) {
>> +    xen_host_pci_config_open(d, errp);
>> +    if (*errp) {
>
> I think that errp could be NULL, therefore the right way to do this is:
>
>      Error *err = NULL;
>      foo(arg, &err);
>      if (err) {
>          handle the error...
>          error_propagate(errp, err);
>      }
>
> see the comment at the beginning of include/qapi/error.h.
>

Thanks for reminding, I didn`t see the comment of error.h before, now I 
am aware why lots of people like the style you mentioned. Will fix it in 
next version, also the comments in other patch.

[...]

-- 
Yours Sincerely,

Cao jin