12.01.2021 00:17, Vladimir Sementsov-Ogievskiy wrote:
11.01.2021 19:08, Alberto Garcia wrote:
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;
}
Actually in our new paradigm of avoiding error propagation (as well as void
functions with errp argument), we should first update read_cache_sizes() to
return the value together with setting errp, then we will update
read_cache_sizes() call in qcow2_update_options_prepare() and drop local_err
from qcow2_update_options_prepare() at all.
Which is actually done during the series, so there is no local_err ;)
But otherwise your solution is correct, so you can keep it if you
prefer:
Reviewed-by: Alberto Garcia <be...@igalia.com>
Thanks!
--
Best regards,
Vladimir
