* [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local
@ 2023-10-23 17:50 Thomas Huth
2023-10-24 4:54 ` Markus Armbruster
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2023-10-23 17:50 UTC (permalink / raw)
To: Hanna Reitz, Kevin Wolf, Markus Armbruster, qemu-devel; +Cc: qemu-block
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();
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local
2023-10-23 17:50 [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local Thomas Huth
@ 2023-10-24 4:54 ` Markus Armbruster
2023-10-24 10:37 ` Markus Armbruster
0 siblings, 1 reply; 5+ messages in thread
From: Markus Armbruster @ 2023-10-24 4:54 UTC (permalink / raw)
To: Thomas Huth; +Cc: Hanna Reitz, Kevin Wolf, qemu-devel, qemu-block
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!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local
2023-10-24 4:54 ` Markus Armbruster
@ 2023-10-24 10:37 ` Markus Armbruster
2023-10-24 11:51 ` Thomas Huth
0 siblings, 1 reply; 5+ messages in thread
From: Markus Armbruster @ 2023-10-24 10:37 UTC (permalink / raw)
To: Thomas Huth; +Cc: Hanna Reitz, Kevin Wolf, qemu-devel, qemu-block
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...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local
2023-10-24 10:37 ` Markus Armbruster
@ 2023-10-24 11:51 ` Thomas Huth
2023-11-07 7:55 ` Cédric Le Goater
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2023-10-24 11:51 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Hanna Reitz, Kevin Wolf, qemu-devel, qemu-block
On 24/10/2023 12.37, Markus Armbruster wrote:
> 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...
Fine for me!
Thomas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local
2023-10-24 11:51 ` Thomas Huth
@ 2023-11-07 7:55 ` Cédric Le Goater
0 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2023-11-07 7:55 UTC (permalink / raw)
To: Thomas Huth, Markus Armbruster
Cc: Hanna Reitz, Kevin Wolf, qemu-devel, qemu-block
On 10/24/23 13:51, Thomas Huth wrote:
> On 24/10/2023 12.37, Markus Armbruster wrote:
>> 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...
>
> Fine for me!
This looks like the last remaining warning. Are we going to activate
-Wshadow=local next ?
Thanks,
C.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-07 7:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-23 17:50 [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local Thomas Huth
2023-10-24 4:54 ` Markus Armbruster
2023-10-24 10:37 ` Markus Armbruster
2023-10-24 11:51 ` Thomas Huth
2023-11-07 7:55 ` Cédric Le Goater
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).