[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [v3] Help wanted for enabling -Wshadow=local
From: |
Markus Armbruster |
Subject: |
Re: [v3] Help wanted for enabling -Wshadow=local |
Date: |
Fri, 06 Oct 2023 20:41:20 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Markus Armbruster <armbru@redhat.com> writes:
> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand. Bugs love to hide in such code.
> Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
> on polling error".
>
> Enabling -Wshadow would prevent bugs like this one. But we have to
> clean up all the offenders first.
>
> Quite a few people responded to my calls for help. Thank you so much!
>
> I'm collecting patches in my git repo at
> https://repo.or.cz/qemu/armbru.git in branch shadow-next. All but the
> last two are in a pending pull request.
>
> My test build is down to seven files with warnings. "[PATCH v2 0/3]
> hexagon: GETPC() and shadowing fixes" takes care of four, but it needs a
> rebase.
>
> Remaining three:
>
> In file included from ../hw/display/virtio-gpu-virgl.c:19:
> ../hw/display/virtio-gpu-virgl.c: In function ‘virgl_cmd_submit_3d’:
> /work/armbru/qemu/include/hw/virtio/virtio-gpu.h:228:16: warning:
> declaration of ‘s’ shadows a previous local [-Wshadow=compatible-local]
> 228 | size_t s;
> \
> | ^
> ../hw/display/virtio-gpu-virgl.c:215:5: note: in expansion of macro
> ‘VIRTIO_GPU_FILL_CMD’
> 215 | VIRTIO_GPU_FILL_CMD(cs);
> | ^~~~~~~~~~~~~~~~~~~
> ../hw/display/virtio-gpu-virgl.c:213:12: note: shadowed declaration is
> here
> 213 | size_t s;
> | ^
>
> In file included from ../contrib/vhost-user-gpu/virgl.h:18,
> from ../contrib/vhost-user-gpu/virgl.c:17:
> ../contrib/vhost-user-gpu/virgl.c: In function ‘virgl_cmd_submit_3d’:
> ../contrib/vhost-user-gpu/vugpu.h:167:16: warning: declaration of ‘s’
> shadows a previous local [-Wshadow=compatible-local]
> 167 | size_t s; \
> | ^
> ../contrib/vhost-user-gpu/virgl.c:203:5: note: in expansion of macro
> ‘VUGPU_FILL_CMD’
> 203 | VUGPU_FILL_CMD(cs);
> | ^~~~~~~~~~~~~~
> ../contrib/vhost-user-gpu/virgl.c:201:12: note: shadowed declaration is
> here
> 201 | size_t s;
> | ^
>
> ../contrib/vhost-user-gpu/vhost-user-gpu.c: In function
> ‘vg_resource_flush’:
> ../contrib/vhost-user-gpu/vhost-user-gpu.c:837:29: warning: declaration
> of ‘i’ shadows a previous local [-Wshadow=local]
> 837 | pixman_image_t *i =
> | ^
> ../contrib/vhost-user-gpu/vhost-user-gpu.c:757:9: note: shadowed
> declaration is here
> 757 | int i;
> | ^
>
> Gerd, Marc-André, or anybody else?
Thomas posted patches:
[PATCH] hw/virtio/virtio-gpu: Fix compiler warning when compiling with
-Wshadow
[PATCH] contrib/vhost-user-gpu: Fix compiler warning when compiling with
-Wshadow
> More warnings may lurk in code my test build doesn't compile. Need a
> full CI build with -Wshadow=local to find them. Anybody care to kick
> one off?
Thomas did; see his reply downthread.
Thank you, Thomas!