Re: [PATCH 1/2] hw/nvme: fix incorrect use of errp/local_err

[Markus Armbruster]

Klaus Jensen <its@irrelevant.dk> writes:

> On Nov  9 13:36, Markus Armbruster wrote:
>> Klaus Jensen <its@irrelevant.dk> writes:
>> 
>> > From: Klaus Jensen <k.jensen@samsung.com>
>> >
>> > Make nvme_check_constraints() return an int and fix incorrect use of
>> > errp/local_err.
>> >
>> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
>> 
>> One more question: what exactly do you mean by "incorrect use of
>> errp/local_err"?  Is it incorrect in the sense of "behaves badly", or
>> merely in the sense of "doesn't use the Error API the way it wants to be
>> used"?
>> 
>
> It's the last hunk of the patch:
>
> @@ -7586,7 +7585,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
> **errp)
>                &pci_dev->qdev, n->parent_obj.qdev.id);
>
>      if (nvme_init_subsys(n, errp)) {
> -        error_propagate(errp, local_err);
>          return;
>      }
>
> It propagates local_err (and it's NULL here).

Now I see, thanks!

Harmless, because error_propagate(errp, NULL) does nothing.  Worth
cleaning up all the same.

> And the bug is a consequence of the error-prone use of an unneeded local
> error value.

Yes.  Eliminating unnecessary error_propagate() tends to result in more
concise and clearer code.