* [v3] Help wanted for enabling -Wshadow=local @ 2023-10-06 14:45 Markus Armbruster 2023-10-06 16:18 ` Thomas Huth 2023-10-06 18:41 ` Markus Armbruster 0 siblings, 2 replies; 7+ messages in thread From: Markus Armbruster @ 2023-10-06 14:45 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, Marc-André Lureau 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? 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? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v3] Help wanted for enabling -Wshadow=local 2023-10-06 14:45 [v3] Help wanted for enabling -Wshadow=local Markus Armbruster @ 2023-10-06 16:18 ` Thomas Huth 2023-10-06 17:55 ` Thomas Huth 2023-10-06 18:41 ` Markus Armbruster 1 sibling, 1 reply; 7+ messages in thread From: Thomas Huth @ 2023-10-06 16:18 UTC (permalink / raw) To: Markus Armbruster, qemu-devel; +Cc: Gerd Hoffmann, Marc-André Lureau On 06/10/2023 16.45, Markus Armbruster wrote: > 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? > > 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? I ran a build here (with -Werror enabled, so that it's easier to see where it breaks): https://gitlab.com/thuth/qemu/-/pipelines/1028023489 ... but I didn't see any additional spots in the logs beside the ones that you already listed. Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v3] Help wanted for enabling -Wshadow=local 2023-10-06 16:18 ` Thomas Huth @ 2023-10-06 17:55 ` Thomas Huth 2023-10-06 19:08 ` Warner Losh 0 siblings, 1 reply; 7+ messages in thread From: Thomas Huth @ 2023-10-06 17:55 UTC (permalink / raw) To: Markus Armbruster, qemu-devel Cc: Gerd Hoffmann, Marc-André Lureau, Warner Losh On 06/10/2023 18.18, Thomas Huth wrote: > On 06/10/2023 16.45, Markus Armbruster wrote: >> 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? >> >> 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? > > I ran a build here (with -Werror enabled, so that it's easier to see where > it breaks): > > https://gitlab.com/thuth/qemu/-/pipelines/1028023489 > > ... but I didn't see any additional spots in the logs beside the ones that > you already listed. After adding two more patches to fix the above warnings, things look pretty good: https://gitlab.com/thuth/qemu/-/pipelines/1028413030 There are just some warnings left in the BSD code, as Warner already mentioned in his reply to v2 of your mail: https://gitlab.com/thuth/qemu/-/jobs/5241420713 Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v3] Help wanted for enabling -Wshadow=local 2023-10-06 17:55 ` Thomas Huth @ 2023-10-06 19:08 ` Warner Losh 2023-10-09 6:24 ` Markus Armbruster 2023-10-09 7:50 ` Thomas Huth 0 siblings, 2 replies; 7+ messages in thread From: Warner Losh @ 2023-10-06 19:08 UTC (permalink / raw) To: Thomas Huth Cc: Markus Armbruster, QEMU Developers, Gerd Hoffmann, Marc-André Lureau [-- Attachment #1: Type: text/plain, Size: 4305 bytes --] On Fri, Oct 6, 2023, 11:55 AM Thomas Huth <thuth@redhat.com> wrote: > On 06/10/2023 18.18, Thomas Huth wrote: > > On 06/10/2023 16.45, Markus Armbruster wrote: > >> 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? > >> > >> 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? > > > > I ran a build here (with -Werror enabled, so that it's easier to see > where > > it breaks): > > > > https://gitlab.com/thuth/qemu/-/pipelines/1028023489 > > > > ... but I didn't see any additional spots in the logs beside the ones > that > > you already listed. > > After adding two more patches to fix the above warnings, things look > pretty > good: > > https://gitlab.com/thuth/qemu/-/pipelines/1028413030 > > There are just some warnings left in the BSD code, as Warner already > mentioned in his reply to v2 of your mail: > > https://gitlab.com/thuth/qemu/-/jobs/5241420713 I think I have fixes for these. I need to merge what just landed into bsd-user fork, rebase, test, the apply them to qemu master branch, retest and send them off... My illness has hung on longer than I thought so I'm still behind... Warner > Thomas > > [-- Attachment #2: Type: text/html, Size: 6429 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v3] Help wanted for enabling -Wshadow=local 2023-10-06 19:08 ` Warner Losh @ 2023-10-09 6:24 ` Markus Armbruster 2023-10-09 7:50 ` Thomas Huth 1 sibling, 0 replies; 7+ messages in thread From: Markus Armbruster @ 2023-10-09 6:24 UTC (permalink / raw) To: Warner Losh Cc: Thomas Huth, QEMU Developers, Gerd Hoffmann, Marc-André Lureau Warner Losh <imp@bsdimp.com> writes: > On Fri, Oct 6, 2023, 11:55 AM Thomas Huth <thuth@redhat.com> wrote: > >> On 06/10/2023 18.18, Thomas Huth wrote: >> > On 06/10/2023 16.45, Markus Armbruster wrote: >> >> 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. [...] >> >> 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? >> > >> > I ran a build here (with -Werror enabled, so that it's easier to see where >> > it breaks): >> > >> > https://gitlab.com/thuth/qemu/-/pipelines/1028023489 >> > >> > ... but I didn't see any additional spots in the logs beside the ones that >> > you already listed. >> >> After adding two more patches to fix the above warnings, things look >> pretty >> good: >> >> https://gitlab.com/thuth/qemu/-/pipelines/1028413030 >> >> There are just some warnings left in the BSD code, as Warner already >> mentioned in his reply to v2 of your mail: >> >> https://gitlab.com/thuth/qemu/-/jobs/5241420713 > > > I think I have fixes for these. I need to merge what just landed into > bsd-user fork, rebase, test, the apply them to qemu master branch, retest > and send them off... > > My illness has hung on longer than I thought so I'm still behind... Get well, and looking forward to your patches! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v3] Help wanted for enabling -Wshadow=local 2023-10-06 19:08 ` Warner Losh 2023-10-09 6:24 ` Markus Armbruster @ 2023-10-09 7:50 ` Thomas Huth 1 sibling, 0 replies; 7+ messages in thread From: Thomas Huth @ 2023-10-09 7:50 UTC (permalink / raw) To: Warner Losh Cc: Markus Armbruster, QEMU Developers, Gerd Hoffmann, Marc-André Lureau On 06/10/2023 21.08, Warner Losh wrote: > > > On Fri, Oct 6, 2023, 11:55 AM Thomas Huth <thuth@redhat.com > <mailto:thuth@redhat.com>> wrote: > > On 06/10/2023 18.18, Thomas Huth wrote: > > On 06/10/2023 16.45, Markus Armbruster wrote: > >> 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 > <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? > >> > >> 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? > > > > I ran a build here (with -Werror enabled, so that it's easier to see > where > > it breaks): > > > > https://gitlab.com/thuth/qemu/-/pipelines/1028023489 > <https://gitlab.com/thuth/qemu/-/pipelines/1028023489> > > > > ... but I didn't see any additional spots in the logs beside the ones > that > > you already listed. > > After adding two more patches to fix the above warnings, things look pretty > good: > > https://gitlab.com/thuth/qemu/-/pipelines/1028413030 > <https://gitlab.com/thuth/qemu/-/pipelines/1028413030> > > There are just some warnings left in the BSD code, as Warner already > mentioned in his reply to v2 of your mail: > > https://gitlab.com/thuth/qemu/-/jobs/5241420713 > <https://gitlab.com/thuth/qemu/-/jobs/5241420713> > > > I think I have fixes for these. I need to merge what just landed into > bsd-user fork, rebase, test, the apply them to qemu master branch, retest > and send them off... > > My illness has hung on longer than I thought so I'm still behind... Get well soon again! ... and no worries about the -Wshadow=local patches in the BSD code, there is no hurry - The BSDs are using Clang by default, so that option won't get enabled by default there anyway yet - I had to switch to GCC in the CI pipeline to trigger those, and I guess only very few people will use GCC to compile QEMU on FreeBSD. Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v3] Help wanted for enabling -Wshadow=local 2023-10-06 14:45 [v3] Help wanted for enabling -Wshadow=local Markus Armbruster 2023-10-06 16:18 ` Thomas Huth @ 2023-10-06 18:41 ` Markus Armbruster 1 sibling, 0 replies; 7+ messages in thread From: Markus Armbruster @ 2023-10-06 18:41 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, Marc-André Lureau 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! ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-09 7:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-06 14:45 [v3] Help wanted for enabling -Wshadow=local Markus Armbruster 2023-10-06 16:18 ` Thomas Huth 2023-10-06 17:55 ` Thomas Huth 2023-10-06 19:08 ` Warner Losh 2023-10-09 6:24 ` Markus Armbruster 2023-10-09 7:50 ` Thomas Huth 2023-10-06 18:41 ` Markus Armbruster
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).