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.f...@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.
Hi stefano,
I read that comment, and find something maybe new:
"errp could be NULL", I think it is saying, if we are in a .realize()
function, yes, *errp* maybe NULL, but reality is, here is the callee of
.realize(), and we defined a local variable: Error *local_err = NULL in
.realize() and passed it to all the callee, so, theoretically *errp*
won`t be NULL. so the way you said above is suitable in .realize() IMHO,
and I also did it in that way.
comment also says:
* Receive an error and pass it on to the caller:
* Error *err = NULL;
* foo(arg, &err);
* if (err) {
* handle the error...
* error_propagate(errp, err);
* }
* where Error **errp is a parameter, by convention the last one.
If I understand the last sentence well, the Error **errp in .realize()
prototype is *the last one*, so we could call error_propagate(errp, err)
only in .realize()
The comment also says:
* But when all you do with the error is pass it on, please use
* foo(arg, errp);
* for readability."
We just pass error on in all the callees, so I guess I also did as
comment suggest?
How do you think?
[...]
--
Yours Sincerely,
Cao Jin
