[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6] nbd: well form nbd_iter_channel_error errp handler
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v6] nbd: well form nbd_iter_channel_error errp handler |
Date: |
Fri, 29 Nov 2019 14:17:35 +0000 |
29.11.2019 16:25, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <address@hidden> writes:
>
>> Make nbd_iter_channel_error errp handler well formed:
>> rename local_err to errp_in, as it is IN-parameter here (which is
>> unusual for Error**).
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> ---
>>
>> v6: fix commit message
>> add Eric's r-b
>>
>> block/nbd.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 5f18f78a94..345bf902e3 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -864,18 +864,18 @@ typedef struct NBDReplyChunkIter {
>> } NBDReplyChunkIter;
>>
>> static void nbd_iter_channel_error(NBDReplyChunkIter *iter,
>> - int ret, Error **local_err)
>> + int ret, Error **errp_in)
>> {
>> - assert(ret < 0);
>> + assert(ret < 0 && errp_in && *errp_in);
>>
>> if (!iter->ret) {
>> iter->ret = ret;
>> - error_propagate(&iter->err, *local_err);
>> + error_propagate(&iter->err, *errp_in);
>> } else {
>> - error_free(*local_err);
>> + error_free(*errp_in);
>> }
>>
>> - *local_err = NULL;
>> + *errp_in = NULL;
>
> This one is actually in/out.
>
> If we use the convention
>
> Any Error ** parameter meant for passing an error to the caller must
> be named @errp. No other Error ** parameter may be named @errp.
>
> then the old name is as good as the new one. But the new one's "in"
> suggestion is misleading.
>
Agreed. Do you have a suggestion how to rename errp in such cases (using
local_err in general will be misleading too)..
Maybe, "filled_errp" ? Seems too long..
"set_errp" is shorter, but no one will guess that this is the third form of the
verb..
>> }
>>
>> static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
>
--
Best regards,
Vladimir