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

Reply via email to