Kevin Wolf <kw...@redhat.com> writes:
> Am 19.09.2023 um 07:48 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kw...@redhat.com> writes:
>>
>> > Am 31.08.2023 um 15:25 hat Markus Armbruster geschrieben:
>> >> Local variables shadowing other local variables or parameters make the
>> >> code needlessly hard to understand. Tracked down with -Wshadow=local.
>> >> Clean up: delete inner declarations when they are actually redundant,
>> >> else rename variables.
>> >>
>> >> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> >> ---
>> >> block/monitor/bitmap-qmp-cmds.c | 2 +-
>> >> block/qcow2-bitmap.c | 3 +--
>> >> 2 files changed, 2 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/block/monitor/bitmap-qmp-cmds.c
>> >> b/block/monitor/bitmap-qmp-cmds.c
>> >> index 55f778f5af..4d018423d8 100644
>> >> --- a/block/monitor/bitmap-qmp-cmds.c
>> >> +++ b/block/monitor/bitmap-qmp-cmds.c
>> >> @@ -276,7 +276,7 @@ BdrvDirtyBitmap *block_dirty_bitmap_merge(const char
>> >> *node, const char *target,
>> >>
>> >> for (lst = bms; lst; lst = lst->next) {
>> >> switch (lst->value->type) {
>> >> - const char *name, *node;
>> >> + const char *name;
>> >> case QTYPE_QSTRING:
>> >> name = lst->value->u.local;
>> >> src = bdrv_find_dirty_bitmap(bs, name);
>> >
>> > The names in this function are all over the place... A more ambitious
>> > patch could rename the parameters to dst_node/dst_bitmap and these
>> > variables to src_node/src_bitmap to get some more consistency (both with
>> > each other and with the existing src/dst variables).
>>
>> What exactly would you like me to consider? Perhaps:
>>
>> * Rename parameter @node to @dst_node
>>
>> * Rename which parameter to @dst_bitmap?
>
> Parameter @target to @dst_bitmap (it's the name of a bitmap in
> @node/@dst_node)
>
>> * Rename nested local @node to @src_node
>>
>> * Rename which local variable to @src_bitmap?
>
> @name to @src_bitmap (it's the name of a bitmap in the local
> @node/@src_node)
>
>> * Move nested locals to function scope
>
> I don't really mind either way, but yes, maybe that would be more
> conventional.
Done for v2.
> That you couldn't tell for two of the variables what they actually are
> probably supports the argument that they should be renamed. :-)
Fair point!
