| Message ID | 20200730005438.138369-1-brogers@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-mem: Work around format specifier mismatch for RISC-V | expand |
On Wed, Jul 29, 2020 at 06:54:38PM -0600, Bruce Rogers wrote: > This likely affects other, less popular host architectures as well. > Less common host architectures under linux get QEMU_VMALLOC_ALIGN (from > which VIRTIO_MEM_MIN_BLOCK_SIZE is derived) define to a variable of > type uintptr, which isn't compatible with the format specifier used to > print a user message. Since this particular usage of the underlying data > seems unique, the simple fix is to just cast it to the corresponding > format specifier. > > Signed-off-by: Bruce Rogers <brogers@suse.com> > --- > hw/virtio/virtio-mem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c > index c12e9f79b0..fd01ffd83e 100644 > --- a/hw/virtio/virtio-mem.c > +++ b/hw/virtio/virtio-mem.c > @@ -754,7 +754,7 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name, > > if (value < VIRTIO_MEM_MIN_BLOCK_SIZE) { > error_setg(errp, "'%s' property has to be at least 0x%" PRIx32, name, > - VIRTIO_MEM_MIN_BLOCK_SIZE); > + (unsigned int)VIRTIO_MEM_MIN_BLOCK_SIZE); Since we use PRIx32, could be better to cast VIRTIO_MEM_MIN_BLOCK_SIZE to uint32_t? Thanks, Stefano > return; > } else if (!is_power_of_2(value)) { > error_setg(errp, "'%s' property has to be a power of two", name); > -- > 2.27.0 > >
On 30.07.20 09:49, Stefano Garzarella wrote: > On Wed, Jul 29, 2020 at 06:54:38PM -0600, Bruce Rogers wrote: >> This likely affects other, less popular host architectures as well. >> Less common host architectures under linux get QEMU_VMALLOC_ALIGN (from >> which VIRTIO_MEM_MIN_BLOCK_SIZE is derived) define to a variable of >> type uintptr, which isn't compatible with the format specifier used to >> print a user message. Since this particular usage of the underlying data >> seems unique, the simple fix is to just cast it to the corresponding >> format specifier. >> >> Signed-off-by: Bruce Rogers <brogers@suse.com> >> --- >> hw/virtio/virtio-mem.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c >> index c12e9f79b0..fd01ffd83e 100644 >> --- a/hw/virtio/virtio-mem.c >> +++ b/hw/virtio/virtio-mem.c >> @@ -754,7 +754,7 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name, >> >> if (value < VIRTIO_MEM_MIN_BLOCK_SIZE) { >> error_setg(errp, "'%s' property has to be at least 0x%" PRIx32, name, >> - VIRTIO_MEM_MIN_BLOCK_SIZE); >> + (unsigned int)VIRTIO_MEM_MIN_BLOCK_SIZE); > > Since we use PRIx32, could be better to cast VIRTIO_MEM_MIN_BLOCK_SIZE > to uint32_t? Yeah, I guess something like -#define VIRTIO_MEM_MIN_BLOCK_SIZE QEMU_VMALLOC_ALIGN +#define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)QEMU_VMALLOC_ALIGN) would be cleaner
On Thu, Jul 30, 2020 at 09:51:19AM +0200, David Hildenbrand wrote: > On 30.07.20 09:49, Stefano Garzarella wrote: > > On Wed, Jul 29, 2020 at 06:54:38PM -0600, Bruce Rogers wrote: > >> This likely affects other, less popular host architectures as well. > >> Less common host architectures under linux get QEMU_VMALLOC_ALIGN (from > >> which VIRTIO_MEM_MIN_BLOCK_SIZE is derived) define to a variable of > >> type uintptr, which isn't compatible with the format specifier used to > >> print a user message. Since this particular usage of the underlying data > >> seems unique, the simple fix is to just cast it to the corresponding > >> format specifier. > >> > >> Signed-off-by: Bruce Rogers <brogers@suse.com> > >> --- > >> hw/virtio/virtio-mem.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c > >> index c12e9f79b0..fd01ffd83e 100644 > >> --- a/hw/virtio/virtio-mem.c > >> +++ b/hw/virtio/virtio-mem.c > >> @@ -754,7 +754,7 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name, > >> > >> if (value < VIRTIO_MEM_MIN_BLOCK_SIZE) { > >> error_setg(errp, "'%s' property has to be at least 0x%" PRIx32, name, > >> - VIRTIO_MEM_MIN_BLOCK_SIZE); > >> + (unsigned int)VIRTIO_MEM_MIN_BLOCK_SIZE); > > > > Since we use PRIx32, could be better to cast VIRTIO_MEM_MIN_BLOCK_SIZE > > to uint32_t? > > Yeah, I guess something like > > -#define VIRTIO_MEM_MIN_BLOCK_SIZE QEMU_VMALLOC_ALIGN > +#define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)QEMU_VMALLOC_ALIGN) > > would be cleaner Yeah, it is cleaner. Otherwise we could use PRIxPTR in the error message. Thanks, Stefano
On 30.07.20 09:58, Stefano Garzarella wrote: > On Thu, Jul 30, 2020 at 09:51:19AM +0200, David Hildenbrand wrote: >> On 30.07.20 09:49, Stefano Garzarella wrote: >>> On Wed, Jul 29, 2020 at 06:54:38PM -0600, Bruce Rogers wrote: >>>> This likely affects other, less popular host architectures as well. >>>> Less common host architectures under linux get QEMU_VMALLOC_ALIGN (from >>>> which VIRTIO_MEM_MIN_BLOCK_SIZE is derived) define to a variable of >>>> type uintptr, which isn't compatible with the format specifier used to >>>> print a user message. Since this particular usage of the underlying data >>>> seems unique, the simple fix is to just cast it to the corresponding >>>> format specifier. >>>> >>>> Signed-off-by: Bruce Rogers <brogers@suse.com> >>>> --- >>>> hw/virtio/virtio-mem.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c >>>> index c12e9f79b0..fd01ffd83e 100644 >>>> --- a/hw/virtio/virtio-mem.c >>>> +++ b/hw/virtio/virtio-mem.c >>>> @@ -754,7 +754,7 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name, >>>> >>>> if (value < VIRTIO_MEM_MIN_BLOCK_SIZE) { >>>> error_setg(errp, "'%s' property has to be at least 0x%" PRIx32, name, >>>> - VIRTIO_MEM_MIN_BLOCK_SIZE); >>>> + (unsigned int)VIRTIO_MEM_MIN_BLOCK_SIZE); >>> >>> Since we use PRIx32, could be better to cast VIRTIO_MEM_MIN_BLOCK_SIZE >>> to uint32_t? >> >> Yeah, I guess something like >> >> -#define VIRTIO_MEM_MIN_BLOCK_SIZE QEMU_VMALLOC_ALIGN >> +#define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)QEMU_VMALLOC_ALIGN) >> >> would be cleaner > > Yeah, it is cleaner. Bruce, can you respin if you agree? Thanks!
On Thu, 2020-07-30 at 10:12 +0200, David Hildenbrand wrote: > On 30.07.20 09:58, Stefano Garzarella wrote: > > On Thu, Jul 30, 2020 at 09:51:19AM +0200, David Hildenbrand wrote: > > > On 30.07.20 09:49, Stefano Garzarella wrote: > > > > On Wed, Jul 29, 2020 at 06:54:38PM -0600, Bruce Rogers wrote: > > > > > This likely affects other, less popular host architectures as > > > > > well. > > > > > Less common host architectures under linux get > > > > > QEMU_VMALLOC_ALIGN (from > > > > > which VIRTIO_MEM_MIN_BLOCK_SIZE is derived) define to a > > > > > variable of > > > > > type uintptr, which isn't compatible with the format > > > > > specifier used to > > > > > print a user message. Since this particular usage of the > > > > > underlying data > > > > > seems unique, the simple fix is to just cast it to the > > > > > corresponding > > > > > format specifier. > > > > > > > > > > Signed-off-by: Bruce Rogers <brogers@suse.com> > > > > > --- > > > > > hw/virtio/virtio-mem.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c > > > > > index c12e9f79b0..fd01ffd83e 100644 > > > > > --- a/hw/virtio/virtio-mem.c > > > > > +++ b/hw/virtio/virtio-mem.c > > > > > @@ -754,7 +754,7 @@ static void > > > > > virtio_mem_set_block_size(Object *obj, Visitor *v, const char > > > > > *name, > > > > > > > > > > if (value < VIRTIO_MEM_MIN_BLOCK_SIZE) { > > > > > error_setg(errp, "'%s' property has to be at least > > > > > 0x%" PRIx32, name, > > > > > - VIRTIO_MEM_MIN_BLOCK_SIZE); > > > > > + (unsigned int)VIRTIO_MEM_MIN_BLOCK_SIZE); > > > > > > > > Since we use PRIx32, could be better to cast > > > > VIRTIO_MEM_MIN_BLOCK_SIZE > > > > to uint32_t? > > > > > > Yeah, I guess something like > > > > > > -#define VIRTIO_MEM_MIN_BLOCK_SIZE QEMU_VMALLOC_ALIGN > > > +#define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)QEMU_VMALLOC_ALIGN) > > > > > > would be cleaner > > > > Yeah, it is cleaner. > > Bruce, can you respin if you agree? Thanks! > Agreed. - Bruce
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index c12e9f79b0..fd01ffd83e 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -754,7 +754,7 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name, if (value < VIRTIO_MEM_MIN_BLOCK_SIZE) { error_setg(errp, "'%s' property has to be at least 0x%" PRIx32, name, - VIRTIO_MEM_MIN_BLOCK_SIZE); + (unsigned int)VIRTIO_MEM_MIN_BLOCK_SIZE); return; } else if (!is_power_of_2(value)) { error_setg(errp, "'%s' property has to be a power of two", name);
This likely affects other, less popular host architectures as well. Less common host architectures under linux get QEMU_VMALLOC_ALIGN (from which VIRTIO_MEM_MIN_BLOCK_SIZE is derived) define to a variable of type uintptr, which isn't compatible with the format specifier used to print a user message. Since this particular usage of the underlying data seems unique, the simple fix is to just cast it to the corresponding format specifier. Signed-off-by: Bruce Rogers <brogers@suse.com> --- hw/virtio/virtio-mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)