Re: [PATCH 5/7] block/vdi: Clean up local variable shadowing

[Markus Armbruster <armbru@xxxxxxxxxx>]

Kevin Wolf <kwolf@xxxxxxxxxx> 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 <armbru@xxxxxxxxxx>
>
>> @@ -700,7 +699,7 @@ nonallocating_write:
>>          /* One or more new blocks were allocated. */
>>          VdiHeader *header;
>>          uint8_t *base;
>> -        uint64_t offset;
>> +        uint64_t offs;
>>          uint32_t n_sectors;
>>  
>>          g_free(block);
>> @@ -723,11 +722,11 @@ nonallocating_write:
>>          bmap_first /= (SECTOR_SIZE / sizeof(uint32_t));
>>          bmap_last /= (SECTOR_SIZE / sizeof(uint32_t));
>>          n_sectors = bmap_last - bmap_first + 1;
>> -        offset = s->bmap_sector + bmap_first;
>> +        offs = s->bmap_sector + bmap_first;
>>          base = ((uint8_t *)&s->bmap[0]) + bmap_first * SECTOR_SIZE;
>>          logout("will write %u block map sectors starting from entry %u\n",
>>                 n_sectors, bmap_first);
>> -        ret = bdrv_co_pwrite(bs->file, offset * SECTOR_SIZE,
>> +        ret = bdrv_co_pwrite(bs->file, offs * SECTOR_SIZE,
>>                               n_sectors * SECTOR_SIZE, base, 0);
>>      }
>
> Having two variables 'offset' and 'offs' doesn't really help with
> clarity either. Can we be more specific and use something like
> 'bmap_offset' here?

Sure!