[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 14/14] block/qcow2: refactor qcow2_update_options_prepare

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

     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

Reviewed-by: Alberto Garcia <berto@igalia.com>


Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]