Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: > 06.12.2019 18:58, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> 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 <vsement...@virtuozzo.com> >>>> --- >>>> 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 <arm...@redhat.com>