* [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 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
* 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
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).