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

[Cao jin]

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 <address@hidden>
>
> 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