Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers

[Markus Armbruster <armbru@redhat.com>]

Cao jin <caoj.fnst@cn.fujitsu.com> writes:

> On 03/02/2016 05:13 PM, Markus Armbruster wrote:
>> This got lost over the Christmas break, sorry.
>>
>> Cc'ing Marcel for additional PCI expertise.
>>
>> Cao jin <caoj.fnst@cn.fujitsu.com> writes:
>>
>>> msi_init() is a supporting function in PCI device initialization,
>>> in order to convert .init() to .realize(), it should be modified first.
>>
>> "Supporting function" doesn't imply "should use Error to report
>> errors".  HACKING explains:
>>
>>      Use the simplest suitable method to communicate success / failure to
>>      callers.  Stick to common methods: non-negative on success / -1 on
>>      error, non-negative / -errno, non-null / null, or Error objects.
>>
>>      Example: when a function returns a non-null pointer on success, and it
>>      can fail only in one way (as far as the caller is concerned), returning
>>      null on failure is just fine, and certainly simpler and a lot easier on
>>      the eyes than propagating an Error object through an Error ** parameter.
>>
>>      Example: when a function's callers need to report details on failure
>>      only the function really knows, use Error **, and set suitable errors.
>>
>>      Do not report an error to the user when you're also returning an error
>>      for somebody else to handle.  Leave the reporting to the place that
>>      consumes the error returned.
>>
>
> Really appreciate your review, I just finished reading all the
> comments and discussion.
>
> Seems pci_add_capability2()(commit cd9aa33e introduced) doesn`t follow
> the new error reporting rule(report error while also return error).

Misunderstanding?

"Report an error to the user" means error_report() and such.
error_setg() doesn't report to the user, it returns an error object to
the caller.

> So I am thinking, could we revert commit cd9aa33e, let
> pci_add_capability() return error code and assert when out of pci
> space, and let caller(only assigned device, others could ignore the
> error) handle the error code(new a error object, propagate it)
>
> Hope to hear PCI Maintainer`s advice(So I don`t cc other in this round)