|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [PATCH v5 14/14] block/qcow2: refactor qcow2_update_options_prepare error paths |
Date: | Tue, 12 Jan 2021 00:17:41 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
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 <vsementsov@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.
But otherwise your solution is correct, so you can keep it if you prefer: Reviewed-by: Alberto Garcia <berto@igalia.com>
Thanks! -- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |