contrib/vhost-user-gpu/vugpu.h | 8 ++++---- contrib/vhost-user-gpu/vhost-user-gpu.c | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)
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)
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.0Thomas 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!
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.
© 2016 - 2024 Red Hat, Inc.