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

[Cao jin <caoj.fnst@cn.fujitsu.com>]

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).

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)

-- 
Yours Sincerely,

Cao jin