[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & mo
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers |
Date: |
Wed, 23 Mar 2016 09:12:39 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Cao jin <address@hidden> 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 <address@hidden> 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)
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, (continued)
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Marcel Apfelbaum, 2016/03/03
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Michael S. Tsirkin, 2016/03/03
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Markus Armbruster, 2016/03/03
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Michael S. Tsirkin, 2016/03/03
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Markus Armbruster, 2016/03/04
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Michael S. Tsirkin, 2016/03/04
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Markus Armbruster, 2016/03/04
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Michael S. Tsirkin, 2016/03/04
Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Markus Armbruster, 2016/03/03
Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers, Cao jin, 2016/03/23
- Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers,
Markus Armbruster <=