29.11.2019 16:25, Markus Armbruster wrote: > Vladimir Sementsov-Ogievskiy <[email protected]> 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 <[email protected]> >> Reviewed-by: Eric Blake <[email protected]> >> --- >> >> 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
