Re: [PATCH 5/7] block/vdi: Clean up local variable shadowing
Kevin Wolf 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 > >> @@ -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 *)>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!
Re: [PATCH 5/7] block/vdi: Clean up local variable shadowing
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 > @@ -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 *)>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? Kevin
Re: [PATCH 5/7] block/vdi: Clean up local variable shadowing
On Thu, Aug 31, 2023 at 03:25:44PM +0200, Markus Armbruster 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 > --- > block/vdi.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature