On Sat 09 Jan 2021 01:58:11 PM CET, Vladimir Sementsov-Ogievskiy wrote: > Keep setting ret close to setting errp and don't merge different error > paths into one. This way it's more obvious that we don't return > error without setting errp. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
I get the idea, but I feel the code is getting innecessarily verbose. One alternative would be to get rid of all -EINVAL inside the switch block, take advantage of the existing local_err and follow the block with: if (local_err) { error_propagate(errp, local_err); ret = -EINVAL; goto fail; } But otherwise your solution is correct, so you can keep it if you prefer: Reviewed-by: Alberto Garcia <be...@igalia.com> Berto