[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 21/21] nbd: assert that Error** is not NULL in nbd_iter_ch
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v8 21/21] nbd: assert that Error** is not NULL in nbd_iter_channel_error |
Date: |
Tue, 10 Dec 2019 10:05:48 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Vladimir Sementsov-Ogievskiy <address@hidden> writes:
> 06.12.2019 18:58, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> On 12/5/19 11:46 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>> The local_err parameter is not here to return information about
>>>> nbd_iter_channel_error failure. Instead it's assumed to be filled when
>>>> passed to the function. This is already stressed by its name
>>>> (local_err, instead of classic errp). Stress it additionally by
>>>> assertion.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>>> ---
>>>> block/nbd.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>
>>> Our timing resulted in crossed mails - I was replying to v7 when this
>>> landed, and my reply there is still relevant (namely, a better commit
>>> message is needed, but the code gets my R-b with that change).
>>
>> If v8 turns out to be fine except for commit message tweaks, I'll gladly
>> to these in my tree. Just tell me what to do in a reply to this
>> message.
>>
>
> Yes, this would be great, thanks!
>
> The only thing is your suggestion on patch 21, but it may be applied in
> separate (and it's actually a separate thing)
It's closer to idea than to suggestion, and it's separate.
Commit message in my tree:
nbd: assert that Error** is not NULL in nbd_iter_channel_error
All callers of nbd_iter_channel_error() pass the address of a
local_err variable, and only call this function if an error has
already occurred, using this function to propagate that error.
This is already implied by its name (local_err instead of the classic
errp), but it is worth additionally stressing this by adding an
assertion to make it part of the function contract.
The local_err parameter is not here to return information about
nbd_iter_channel_error failure. Instead it's assumed to be filled when
passed to the function. This is already stressed by its name
(local_err, instead of classic errp). Stress it additionally by
assertion.
Also:
Reviewed-by: Markus Armbruster <address@hidden>
- Re: [PATCH v8 06/21] qdev-monitor: well form error hint helpers, (continued)
[PATCH v8 01/21] hw/core/loader-fit: fix freeing errp in fit_load_fdt, Vladimir Sementsov-Ogievskiy, 2019/12/05
[PATCH v8 19/21] backends/cryptodev: drop local_err from cryptodev_backend_complete(), Vladimir Sementsov-Ogievskiy, 2019/12/05
[PATCH v8 16/21] hw/tpm: rename Error ** parameter to more common errp, Vladimir Sementsov-Ogievskiy, 2019/12/05
[PATCH v8 03/21] error: make Error **errp const where it is appropriate, Vladimir Sementsov-Ogievskiy, 2019/12/05
[PATCH v8 12/21] qga: rename Error ** parameter to more common errp, Vladimir Sementsov-Ogievskiy, 2019/12/05