| Message ID | 20230904161235.84651-23-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | (few more) Steps towards enabling -Wshadow | expand |
On Mon, Sep 04, 2023 at 06:12:34PM +0200, Philippe Mathieu-Daudé wrote: > Fix: > > softmmu/physmem.c: In function ‘cpu_physical_memory_snapshot_and_clear_dirty’: > softmmu/physmem.c:916:27: warning: declaration of ‘offset’ shadows a parameter [-Wshadow=compatible-local] > 916 | unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; > | ^~~~~~ > softmmu/physmem.c:892:31: note: shadowed declaration is here > 892 | (MemoryRegion *mr, hwaddr offset, hwaddr length, unsigned client) > | ~~~~~~~^~~~~~ > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > RFC: Please double-check how 'offset' is used few lines later. I don't see an issue - those lines are in an outer scope, so won't be accessing the 'offset' you've changed, they'll be the parameter instead. If you want to sanity check though, presumably the asm dissassembly for this method should be the same before/after this change > --- > softmmu/physmem.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > diff --git a/softmmu/physmem.c b/softmmu/physmem.c > index 18277ddd67..db5b628a60 100644 > --- a/softmmu/physmem.c > +++ b/softmmu/physmem.c > @@ -913,16 +913,16 @@ DirtyBitmapSnapshot *cpu_physical_memory_snapshot_and_clear_dirty > > while (page < end) { > unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE; > - unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; > + unsigned long ofs = page % DIRTY_MEMORY_BLOCK_SIZE; > unsigned long num = MIN(end - page, > - DIRTY_MEMORY_BLOCK_SIZE - offset); > + DIRTY_MEMORY_BLOCK_SIZE - ofs); > > - assert(QEMU_IS_ALIGNED(offset, (1 << BITS_PER_LEVEL))); > + assert(QEMU_IS_ALIGNED(ofs, (1 << BITS_PER_LEVEL))); > assert(QEMU_IS_ALIGNED(num, (1 << BITS_PER_LEVEL))); > - offset >>= BITS_PER_LEVEL; > + ofs >>= BITS_PER_LEVEL; > > bitmap_copy_and_clear_atomic(snap->dirty + dest, > - blocks->blocks[idx] + offset, > + blocks->blocks[idx] + ofs, > num); > page += num; > dest += num >> BITS_PER_LEVEL; > -- > 2.41.0 > > With regards, Daniel
On Mon, Sep 04, 2023 at 05:31:30PM +0100, Daniel P. Berrangé wrote: > On Mon, Sep 04, 2023 at 06:12:34PM +0200, Philippe Mathieu-Daudé wrote: > > Fix: > > > > softmmu/physmem.c: In function ‘cpu_physical_memory_snapshot_and_clear_dirty’: > > softmmu/physmem.c:916:27: warning: declaration of ‘offset’ shadows a parameter [-Wshadow=compatible-local] > > 916 | unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; > > | ^~~~~~ > > softmmu/physmem.c:892:31: note: shadowed declaration is here > > 892 | (MemoryRegion *mr, hwaddr offset, hwaddr length, unsigned client) > > | ~~~~~~~^~~~~~ > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > --- > > RFC: Please double-check how 'offset' is used few lines later. > > I don't see an issue - those lines are in an outer scope, so won't > be accessing the 'offset' you've changed, they'll be the parameter > instead. If you want to sanity check though, presumably the asm > dissassembly for this method should be the same before/after this > change (and if it didn't do so then it's a bug..) > > > --- > > softmmu/physmem.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com>
diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 18277ddd67..db5b628a60 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -913,16 +913,16 @@ DirtyBitmapSnapshot *cpu_physical_memory_snapshot_and_clear_dirty while (page < end) { unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE; - unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; + unsigned long ofs = page % DIRTY_MEMORY_BLOCK_SIZE; unsigned long num = MIN(end - page, - DIRTY_MEMORY_BLOCK_SIZE - offset); + DIRTY_MEMORY_BLOCK_SIZE - ofs); - assert(QEMU_IS_ALIGNED(offset, (1 << BITS_PER_LEVEL))); + assert(QEMU_IS_ALIGNED(ofs, (1 << BITS_PER_LEVEL))); assert(QEMU_IS_ALIGNED(num, (1 << BITS_PER_LEVEL))); - offset >>= BITS_PER_LEVEL; + ofs >>= BITS_PER_LEVEL; bitmap_copy_and_clear_atomic(snap->dirty + dest, - blocks->blocks[idx] + offset, + blocks->blocks[idx] + ofs, num); page += num; dest += num >> BITS_PER_LEVEL;
Fix: softmmu/physmem.c: In function ‘cpu_physical_memory_snapshot_and_clear_dirty’: softmmu/physmem.c:916:27: warning: declaration of ‘offset’ shadows a parameter [-Wshadow=compatible-local] 916 | unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; | ^~~~~~ softmmu/physmem.c:892:31: note: shadowed declaration is here 892 | (MemoryRegion *mr, hwaddr offset, hwaddr length, unsigned client) | ~~~~~~~^~~~~~ Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- RFC: Please double-check how 'offset' is used few lines later. --- softmmu/physmem.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)