1. Patchew
  2. QEMU
  3. View series

[PATCH] virtio-mem: Work around format specifier mismatch for RISC-V

Bruce Rogers posted 1 patch 3 years, 8 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200730005438.138369-1-brogers@suse.com
Maintainers: David Hildenbrand <david@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
hw/virtio/virtio-mem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] virtio-mem: Work around format specifier mismatch for RISC-V
Posted by Bruce Rogers 3 years, 8 months ago 
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);
         return;
     } else if (!is_power_of_2(value)) {
         error_setg(errp, "'%s' property has to be a power of two", name);
-- 
2.27.0
Re: [PATCH] virtio-mem: Work around format specifier mismatch for RISC-V
Posted by Stefano Garzarella 3 years, 8 months ago 
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
> 
>
Re: [PATCH] virtio-mem: Work around format specifier mismatch for RISC-V
Posted by David Hildenbrand 3 years, 8 months ago 
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

-- 
Thanks,

David / dhildenb
Re: [PATCH] virtio-mem: Work around format specifier mismatch for RISC-V
Posted by Stefano Garzarella 3 years, 8 months ago 
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
Re: [PATCH] virtio-mem: Work around format specifier mismatch for RISC-V
Posted by David Hildenbrand 3 years, 8 months ago 
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!


-- 
Thanks,

David / dhildenb
Re: [PATCH] virtio-mem: Work around format specifier mismatch for RISC-V
Posted by Bruce Rogers 3 years, 8 months ago 
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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.