[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local |
Date: |
Tue, 24 Oct 2023 12:37:13 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Markus Armbruster <armbru@redhat.com> writes:
> Thomas Huth <thuth@redhat.com> writes:
>
>> No need to declare a new variable in the the inner code block
>> here, we can re-use the "ret" variable that has been declared
>> at the beginning of the function. With this change, the code
>> can now be successfully compiled with -Wshadow=local again.
>>
>> Fixes: a32e781838 ("Mark bdrv_snapshot_fallback() and callers GRAPH_RDLOCK")
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> v2: Assign "ret" only in one spot
>>
>> block/snapshot.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/snapshot.c b/block/snapshot.c
>> index 6e16eb803a..55974273ae 100644
>> --- a/block/snapshot.c
>> +++ b/block/snapshot.c
>> @@ -629,7 +629,6 @@ int bdrv_all_goto_snapshot(const char *name,
>> while (iterbdrvs) {
>> BlockDriverState *bs = iterbdrvs->data;
>> AioContext *ctx = bdrv_get_aio_context(bs);
>> - int ret = 0;
>> bool all_snapshots_includes_bs;
>>
>> aio_context_acquire(ctx);
>> @@ -637,9 +636,8 @@ int bdrv_all_goto_snapshot(const char *name,
>> all_snapshots_includes_bs = bdrv_all_snapshots_includes_bs(bs);
>> bdrv_graph_rdunlock_main_loop();
>>
>> - if (devices || all_snapshots_includes_bs) {
>> - ret = bdrv_snapshot_goto(bs, name, errp);
>> - }
>> + ret = (devices || all_snapshots_includes_bs) ?
>> + bdrv_snapshot_goto(bs, name, errp) : 0;
>> aio_context_release(ctx);
>> if (ret < 0) {
>> bdrv_graph_rdlock_main_loop();
>
> Better. Unconditional assignment to @ret right before it's checked is
> how we should use @ret.
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> And queued. Thanks!
Mind if I drop the Fixes: tag? Nothing broken until we enable
-Wshadow=local...