[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/7] qdev_prop_parse(): report errors via Error
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 3/7] qdev_prop_parse(): report errors via Error |
Date: |
Tue, 05 Feb 2013 20:20:09 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130108 Thunderbird/10.0.12 |
On 02/05/13 13:20, Luiz Capitulino wrote:
> On Tue, 05 Feb 2013 01:31:14 +0100
> Laszlo Ersek <address@hidden> wrote:
>
>> On 02/04/13 18:27, Luiz Capitulino wrote:
>>>
>>> I usually split this kind of patch the following way:
>>>
>>> 1. add an Error ** argument to the function reporting the error. Callers
>>> are changed to pass NULL for the new argument
>>
>> If the called function already switches to error_set() /
>> error_propagate() at this point, then standing at this patch in a
>> possible bisection, the error message is lost. (The caller doesn't print
>> it *yet*, the callee doesn't print it *any longer*.)
>
> If I got what you meant, the called function shouldn't be using error_set()
> at this point, as it doesn't even take an Error ** argument.
>
>> If, OTOH, the called function still prints the error message here, and
>> doesn't pass it up (only its signature has changed), then:
>>
>>> 2. Handling of the new error is added for each caller in a different
>>> patch (it's OK to group callers when the error handling is the same)
>>
>> at some point there will be callers that need the callee to pass up the
>> error, and other callers (the ones not yet converted) that want the
>> callee not to.
>
> Yes, but it's case-by-case. For example, if the called function prints to
> the monitor or uses fprintf(), then it's OK to keep them until all callers
> are converted.
>
> Now, if the called function reports error with qerror_report() then
> we have different options. You could call qerror_report_err() at some
> point in the call stack, for example.
>
> But, honestly speaking, I think I went too general on my feedback and
> lost sight of the details concerning this patch and I don't my feedback
> is good enough at this point, sorry for that. I'll try to be more
> specific when you respin.
OK, let me post v2 soon. I don't expect it to be final, but hopefully
it'll enable us to discuss it in more targeted details.
Thanks much!
Laszlo
- [Qemu-devel] [PATCH 4/7] qbus_find_recursive(): reorganize, (continued)
[Qemu-devel] [PATCH 3/7] qdev_prop_parse(): report errors via Error, Laszlo Ersek, 2013/02/01
[Qemu-devel] [PATCH 1/7] remove some trailing whitespace, Laszlo Ersek, 2013/02/01
[Qemu-devel] [PATCH 6/7] qbus_find(): report errors via Error, Laszlo Ersek, 2013/02/01
[Qemu-devel] [PATCH 5/7] qbus_find_recursive(): terminate search by name in case of fatal error, Laszlo Ersek, 2013/02/01
[Qemu-devel] [PATCH 7/7] qdev_device_add(): report errors with Error, Laszlo Ersek, 2013/02/01