Cao jin <caoj.f...@cn.fujitsu.com> writes:
> On 03/23/2016 03:26 PM, Wei Jiangang wrote:
>
>>
>> -static int pxb_dev_init_common(PCIDevice *dev, bool pcie)
>> +static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>> {
>> PXBDev *pxb = convert_to_pxb(dev);
>> DeviceState *ds, *bds = NULL;
>> PCIBus *bus;
>> const char *dev_name = NULL;
>> + Error *local_err = NULL;
>>
>
> the preferred style I think, is /*err/
Both styles are in use. I use err myself, but local_err is okay, too.
>> if (pxb->numa_node != NUMA_NODE_UNASSIGNED &&
>> pxb->numa_node >= nb_numa_nodes) {
>> - error_report("Illegal numa node %d.", pxb->numa_node);
>> - return -EINVAL;
>> + error_setg(errp, "Illegal numa node %d", pxb->numa_node);
>> + return;
>
> since we have local /*err/ to avoid null /**errp/ venture, I guess it
> should be used here too.
No, this is just fine.
If errp is null, error_setg() does nothing, which is exactly what we
want.
However:
>> }
>>
>> if (dev->qdev.id && *dev->qdev.id) {
>> @@ -248,7 +244,9 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie)
>>
>> PCI_HOST_BRIDGE(ds)->bus = bus;
>>
>> - if (pxb_register_bus(dev, bus)) {
>> + pxb_register_bus(dev, bus, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> goto err_register_bus;
>> }
>>
When we need to find out whether the callee set an error, we can't use
(possibly null) errp, because *errp isn't safe.
[...]
