Re: [PATCH v2 2/3] hw/usb/hcd-xhci-pci: Abort if setting link property failed

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 24 Aug 2021 at 15:27, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Tue, 24 Aug 2021 at 13:05, Markus Armbruster <armbru@redhat.com> wrote:
> >> When you know that all callers handle errors like &error_fatal does, use
> >> of &error_fatal doesn't produce wrong behavior.  It's still kind of
> >> wrong, because relying on such a non-local argument without a genuine
> >> need is.
> >
> > Not using error_fatal results in quite a bit of extra boilerplate
> > for all those extra explicit "check for failure, print the error
> > message and exit" points. If the MachineState init function took
> > an Error** that might be a mild encouragement to "pass an Error
> > upward rather than exiting"; but it doesn't.
> >
> > Right now we have nearly a thousand instances of error_fatal
> > in the codebase, incidentally.
>
> Use of &error_fatal is clearly superior to accomplishing the same
> behavior the way you describe.
>
> My point was this behavior is usually wrong in functions with an Error
> **errp parameter.

Right, but as Eduardo has noted, only about 8% of our use of
error_fatal is like that. The vast bulk is other cases, so
if you want to call it "kind of wrong" we ought to have a view
of how it could be done otherwise.

-- PMM