| Message ID | 20231009083726.30301-1-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] contrib/vhost-user-gpu: Fix compiler warning when compiling with -Wshadow | expand |
On Mon, Oct 09, 2023 at 10:37:25AM +0200, Thomas Huth wrote: > Rename some variables to avoid compiler warnings when compiling > with -Wshadow=local. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v2: Renamed the variable to something more unique > > contrib/vhost-user-gpu/vugpu.h | 8 ++++---- > contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++--- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h > index 509b679f03..654c392fbb 100644 > --- a/contrib/vhost-user-gpu/vugpu.h > +++ b/contrib/vhost-user-gpu/vugpu.h > @@ -164,12 +164,12 @@ struct virtio_gpu_ctrl_command { > }; > > #define VUGPU_FILL_CMD(out) do { \ > - size_t s; \ > - s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ > + size_t vugpufillcmd_s_ = \ > + iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ > &out, sizeof(out)); \ > - if (s != sizeof(out)) { \ > + if (vugpufillcmd_s_ != sizeof(out)) { \ > g_critical("%s: command size incorrect %zu vs %zu", \ > - __func__, s, sizeof(out)); \ > + __func__, vugpufillcmd_s_, sizeof(out)); \ > return; \ > } \ > } while (0) I think I prefer VUGPU_FILL_CMD_s or VUGPU_FILL_CMD_s_ - makes it clear it's related to a macro. > diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c > index aa304475a0..bb41758e34 100644 > --- a/contrib/vhost-user-gpu/vhost-user-gpu.c > +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c > @@ -834,7 +834,7 @@ vg_resource_flush(VuGpu *g, > .width = width, > .height = height, > }; > - pixman_image_t *i = > + pixman_image_t *img = > pixman_image_create_bits(pixman_image_get_format(res->image), > msg->payload.update.width, > msg->payload.update.height, > @@ -842,11 +842,11 @@ vg_resource_flush(VuGpu *g, > payload.update.data), > width * bpp); > pixman_image_composite(PIXMAN_OP_SRC, > - res->image, NULL, i, > + res->image, NULL, img, > extents->x1, extents->y1, > 0, 0, 0, 0, > width, height); > - pixman_image_unref(i); > + pixman_image_unref(img); > vg_send_msg(g, msg, -1); > g_free(msg); > } > -- > 2.41.0
On 09/10/2023 13.45, Michael S. Tsirkin wrote: > On Mon, Oct 09, 2023 at 10:37:25AM +0200, Thomas Huth wrote: >> Rename some variables to avoid compiler warnings when compiling >> with -Wshadow=local. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> v2: Renamed the variable to something more unique >> >> contrib/vhost-user-gpu/vugpu.h | 8 ++++---- >> contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++--- >> 2 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h >> index 509b679f03..654c392fbb 100644 >> --- a/contrib/vhost-user-gpu/vugpu.h >> +++ b/contrib/vhost-user-gpu/vugpu.h >> @@ -164,12 +164,12 @@ struct virtio_gpu_ctrl_command { >> }; >> >> #define VUGPU_FILL_CMD(out) do { \ >> - size_t s; \ >> - s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ >> + size_t vugpufillcmd_s_ = \ >> + iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ >> &out, sizeof(out)); \ >> - if (s != sizeof(out)) { \ >> + if (vugpufillcmd_s_ != sizeof(out)) { \ >> g_critical("%s: command size incorrect %zu vs %zu", \ >> - __func__, s, sizeof(out)); \ >> + __func__, vugpufillcmd_s_, sizeof(out)); \ >> return; \ >> } \ >> } while (0) > > I think I prefer VUGPU_FILL_CMD_s or VUGPU_FILL_CMD_s_ - makes it clear > it's related to a macro. I have to say that I don't like that ... it's a variable after all, and naming it with capital letters looks rather confusing that helpful to me. I think it should be enough to have the underscore at the end here to make it unique enough. Thomas
Thomas Huth <thuth@redhat.com> writes: > On 09/10/2023 13.45, Michael S. Tsirkin wrote: >> On Mon, Oct 09, 2023 at 10:37:25AM +0200, Thomas Huth wrote: >>> Rename some variables to avoid compiler warnings when compiling >>> with -Wshadow=local. >>> >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> v2: Renamed the variable to something more unique >>> >>> contrib/vhost-user-gpu/vugpu.h | 8 ++++---- >>> contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++--- >>> 2 files changed, 7 insertions(+), 7 deletions(-) >>> >>> diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h >>> index 509b679f03..654c392fbb 100644 >>> --- a/contrib/vhost-user-gpu/vugpu.h >>> +++ b/contrib/vhost-user-gpu/vugpu.h >>> @@ -164,12 +164,12 @@ struct virtio_gpu_ctrl_command { >>> }; >>> #define VUGPU_FILL_CMD(out) do { \ >>> - size_t s; \ >>> - s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ >>> + size_t vugpufillcmd_s_ = \ >>> + iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ >>> &out, sizeof(out)); \ >>> - if (s != sizeof(out)) { \ >>> + if (vugpufillcmd_s_ != sizeof(out)) { \ >>> g_critical("%s: command size incorrect %zu vs %zu", \ >>> - __func__, s, sizeof(out)); \ >>> + __func__, vugpufillcmd_s_, sizeof(out)); \ >>> return; \ >>> } \ >>> } while (0) >> I think I prefer VUGPU_FILL_CMD_s or VUGPU_FILL_CMD_s_ - makes it clear >> it's related to a macro. > > I have to say that I don't like that ... it's a variable after all, and naming it with capital letters looks rather confusing that helpful to me. I think it should be enough to have the underscore at the end here to make it unique enough. Concur. Plenty of precedence in the tree.
Thomas Huth <thuth@redhat.com> writes: > Rename some variables to avoid compiler warnings when compiling > with -Wshadow=local. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v2: Renamed the variable to something more unique > > contrib/vhost-user-gpu/vugpu.h | 8 ++++---- > contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++--- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h > index 509b679f03..654c392fbb 100644 > --- a/contrib/vhost-user-gpu/vugpu.h > +++ b/contrib/vhost-user-gpu/vugpu.h > @@ -164,12 +164,12 @@ struct virtio_gpu_ctrl_command { > }; > > #define VUGPU_FILL_CMD(out) do { \ > - size_t s; \ > - s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ > + size_t vugpufillcmd_s_ = \ > + iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ > &out, sizeof(out)); \ > - if (s != sizeof(out)) { \ > + if (vugpufillcmd_s_ != sizeof(out)) { \ > g_critical("%s: command size incorrect %zu vs %zu", \ > - __func__, s, sizeof(out)); \ > + __func__, vugpufillcmd_s_, sizeof(out)); \ > return; \ > } \ > } while (0) v1 renamed to s_ instead, which I find much easier to read. Michael asked you to change it so it's less likely to break if we pass it a macro that also uses s_. Unlikely to happen, and would fail safe: build breaks. > diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c > index aa304475a0..bb41758e34 100644 > --- a/contrib/vhost-user-gpu/vhost-user-gpu.c > +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c > @@ -834,7 +834,7 @@ vg_resource_flush(VuGpu *g, > .width = width, > .height = height, > }; > - pixman_image_t *i = > + pixman_image_t *img = > pixman_image_create_bits(pixman_image_get_format(res->image), > msg->payload.update.width, > msg->payload.update.height, > @@ -842,11 +842,11 @@ vg_resource_flush(VuGpu *g, > payload.update.data), > width * bpp); > pixman_image_composite(PIXMAN_OP_SRC, > - res->image, NULL, i, > + res->image, NULL, img, > extents->x1, extents->y1, > 0, 0, 0, 0, > width, height); > - pixman_image_unref(i); > + pixman_image_unref(img); > vg_send_msg(g, msg, -1); > g_free(msg); > } I'm going to queue v1. Michael, if you want me to queue v2 instead, or neither of the two, let me know.
On Thu, Oct 12, 2023 at 02:18:44PM +0200, Markus Armbruster wrote: > Thomas Huth <thuth@redhat.com> writes: > > > Rename some variables to avoid compiler warnings when compiling > > with -Wshadow=local. > > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > --- > > v2: Renamed the variable to something more unique > > > > contrib/vhost-user-gpu/vugpu.h | 8 ++++---- > > contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++--- > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h > > index 509b679f03..654c392fbb 100644 > > --- a/contrib/vhost-user-gpu/vugpu.h > > +++ b/contrib/vhost-user-gpu/vugpu.h > > @@ -164,12 +164,12 @@ struct virtio_gpu_ctrl_command { > > }; > > > > #define VUGPU_FILL_CMD(out) do { \ > > - size_t s; \ > > - s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ > > + size_t vugpufillcmd_s_ = \ > > + iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ > > &out, sizeof(out)); \ > > - if (s != sizeof(out)) { \ > > + if (vugpufillcmd_s_ != sizeof(out)) { \ > > g_critical("%s: command size incorrect %zu vs %zu", \ > > - __func__, s, sizeof(out)); \ > > + __func__, vugpufillcmd_s_, sizeof(out)); \ > > return; \ > > } \ > > } while (0) > > v1 renamed to s_ instead, which I find much easier to read. Michael > asked you to change it so it's less likely to break if we pass it a > macro that also uses s_. Unlikely to happen, and would fail safe: build > breaks. > > > diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c > > index aa304475a0..bb41758e34 100644 > > --- a/contrib/vhost-user-gpu/vhost-user-gpu.c > > +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c > > @@ -834,7 +834,7 @@ vg_resource_flush(VuGpu *g, > > .width = width, > > .height = height, > > }; > > - pixman_image_t *i = > > + pixman_image_t *img = > > pixman_image_create_bits(pixman_image_get_format(res->image), > > msg->payload.update.width, > > msg->payload.update.height, > > @@ -842,11 +842,11 @@ vg_resource_flush(VuGpu *g, > > payload.update.data), > > width * bpp); > > pixman_image_composite(PIXMAN_OP_SRC, > > - res->image, NULL, i, > > + res->image, NULL, img, > > extents->x1, extents->y1, > > 0, 0, 0, 0, > > width, height); > > - pixman_image_unref(i); > > + pixman_image_unref(img); > > vg_send_msg(g, msg, -1); > > g_free(msg); > > } > > I'm going to queue v1. Michael, if you want me to queue v2 instead, or > neither of the two, let me know. Yea I think v2 is better, queue that please.
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Thu, Oct 12, 2023 at 02:18:44PM +0200, Markus Armbruster wrote: >> Thomas Huth <thuth@redhat.com> writes: >> >> > Rename some variables to avoid compiler warnings when compiling >> > with -Wshadow=local. >> > >> > Signed-off-by: Thomas Huth <thuth@redhat.com> >> > --- >> > v2: Renamed the variable to something more unique [...] >> v1 renamed to s_ instead, which I find much easier to read. Michael >> asked you to change it so it's less likely to break if we pass it a >> macro that also uses s_. Unlikely to happen, and would fail safe: build >> breaks. [...] >> I'm going to queue v1. Michael, if you want me to queue v2 instead, or >> neither of the two, let me know. > > Yea I think v2 is better, queue that please. Done. Thanks!
diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h index 509b679f03..654c392fbb 100644 --- a/contrib/vhost-user-gpu/vugpu.h +++ b/contrib/vhost-user-gpu/vugpu.h @@ -164,12 +164,12 @@ struct virtio_gpu_ctrl_command { }; #define VUGPU_FILL_CMD(out) do { \ - size_t s; \ - s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ + size_t vugpufillcmd_s_ = \ + iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0, \ &out, sizeof(out)); \ - if (s != sizeof(out)) { \ + if (vugpufillcmd_s_ != sizeof(out)) { \ g_critical("%s: command size incorrect %zu vs %zu", \ - __func__, s, sizeof(out)); \ + __func__, vugpufillcmd_s_, sizeof(out)); \ return; \ } \ } while (0) diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c index aa304475a0..bb41758e34 100644 --- a/contrib/vhost-user-gpu/vhost-user-gpu.c +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c @@ -834,7 +834,7 @@ vg_resource_flush(VuGpu *g, .width = width, .height = height, }; - pixman_image_t *i = + pixman_image_t *img = pixman_image_create_bits(pixman_image_get_format(res->image), msg->payload.update.width, msg->payload.update.height, @@ -842,11 +842,11 @@ vg_resource_flush(VuGpu *g, payload.update.data), width * bpp); pixman_image_composite(PIXMAN_OP_SRC, - res->image, NULL, i, + res->image, NULL, img, extents->x1, extents->y1, 0, 0, 0, 0, width, height); - pixman_image_unref(i); + pixman_image_unref(img); vg_send_msg(g, msg, -1); g_free(msg); }
Rename some variables to avoid compiler warnings when compiling with -Wshadow=local. Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Renamed the variable to something more unique contrib/vhost-user-gpu/vugpu.h | 8 ++++---- contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)