Re: [PATCH 3/7] ui: Clean up local variable shadowing

[Markus Armbruster <armbru@xxxxxxxxxx>]

Peter Maydell <peter.maydell@xxxxxxxxxx> writes:

> On Thu, 31 Aug 2023 at 14:25, Markus Armbruster <armbru@xxxxxxxxxx> wrote:
>>
>> 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 <armbru@xxxxxxxxxx>
>
>
>> diff --git a/ui/vnc-enc-zrle.c.inc b/ui/vnc-enc-zrle.c.inc
>> index c107d8affc..edf42d4a6a 100644
>> --- a/ui/vnc-enc-zrle.c.inc
>> +++ b/ui/vnc-enc-zrle.c.inc
>> @@ -153,11 +153,12 @@ static void ZRLE_ENCODE_TILE(VncState *vs, ZRLE_PIXEL 
>> *data, int w, int h,
>>      }
>>
>>      if (use_rle) {
>> -        ZRLE_PIXEL *ptr = data;
>> -        ZRLE_PIXEL *end = ptr + w * h;
>>          ZRLE_PIXEL *run_start;
>>          ZRLE_PIXEL pix;
>>
>> +        ptr = data;
>> +        end = ptr + w * h;
>> +
>>          while (ptr < end) {
>>              int len;
>>              int index = 0;
>> @@ -198,7 +199,7 @@ static void ZRLE_ENCODE_TILE(VncState *vs, ZRLE_PIXEL 
>> *data, int w, int h,
>>          }
>>      } else if (use_palette) { /* no RLE */
>>          int bppp;
>> -        ZRLE_PIXEL *ptr = data;
>> +        ptr = data;
>>
>>          /* packed pixels */
>>
>> @@ -241,8 +242,6 @@ static void ZRLE_ENCODE_TILE(VncState *vs, ZRLE_PIXEL 
>> *data, int w, int h,
>>  #endif
>>          {
>>  #ifdef ZRLE_COMPACT_PIXEL
>> -            ZRLE_PIXEL *ptr;
>> -
>>              for (ptr = data; ptr < data + w * h; ptr++) {
>>                  ZRLE_WRITE_PIXEL(vs, *ptr);
>>              }
>
> For this one I'm tempted to suggest instead moving the
> pix and end currently at whole-function scope into their
> own block, so it's clear these are actually four
> completely independent uses of ptr/end. But either way

You have a point.  However, we'd need to splice in a block just for
that, which involved some reindenting.  Can do if the assigned
maintainers (Gerd and Marc-André) prefer it.  Else, I'll stay with
minimally invasive.

> Reviewed-by: Peter Maydell <peter.maydell@xxxxxxxxxx>

Thanks!