[Qemu-devel] [PATCH] kvm: Silence warning from valgrind

[Qemu-devel] [PATCH] kvm: Silence warning from valgrind

[Thomas Huth]

valgrind complains here about uninitialized bytes with the following message:

==17814== Syscall param ioctl(generic) points to uninitialised byte(s)
==17814==    at 0x466A780: ioctl (in /usr/lib64/power8/libc-2.17.so)
==17814==    by 0x100735B7: kvm_vm_ioctl (kvm-all.c:1920)
==17814==    by 0x10074583: kvm_set_ioeventfd_mmio (kvm-all.c:574)

Let's fix it by using a proper struct initializer in kvm_set_ioeventfd_mmio().

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 kvm-all.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index dd44f8c..077b0ed 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -552,13 +552,13 @@ static int kvm_set_ioeventfd_mmio(int fd, hwaddr addr, uint32_t val,
                                   bool assign, uint32_t size, bool datamatch)
 {
     int ret;
-    struct kvm_ioeventfd iofd;
-
-    iofd.datamatch = datamatch ? adjust_ioeventfd_endianness(val, size) : 0;
-    iofd.addr = addr;
-    iofd.len = size;
-    iofd.flags = 0;
-    iofd.fd = fd;
+    struct kvm_ioeventfd iofd = {
+        .datamatch = datamatch ? adjust_ioeventfd_endianness(val, size) : 0,
+        .addr = addr,
+        .len = size,
+        .flags = 0,
+        .fd = fd,
+    };
 
     if (!kvm_enabled()) {
         return -ENOSYS;
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-trivial] [PATCH] kvm: Silence warning from valgrind

[Michael Tokarev]

27.04.2015 19:59, Thomas Huth wrote:
> valgrind complains here about uninitialized bytes with the following message:
> 
> ==17814== Syscall param ioctl(generic) points to uninitialised byte(s)
> ==17814==    at 0x466A780: ioctl (in /usr/lib64/power8/libc-2.17.so)
> ==17814==    by 0x100735B7: kvm_vm_ioctl (kvm-all.c:1920)
> ==17814==    by 0x10074583: kvm_set_ioeventfd_mmio (kvm-all.c:574)
> 
> Let's fix it by using a proper struct initializer in kvm_set_ioeventfd_mmio().
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  kvm-all.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index dd44f8c..077b0ed 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -552,13 +552,13 @@ static int kvm_set_ioeventfd_mmio(int fd, hwaddr addr, uint32_t val,
>                                    bool assign, uint32_t size, bool datamatch)
>  {
>      int ret;
> -    struct kvm_ioeventfd iofd;
> -
> -    iofd.datamatch = datamatch ? adjust_ioeventfd_endianness(val, size) : 0;
> -    iofd.addr = addr;
> -    iofd.len = size;
> -    iofd.flags = 0;
> -    iofd.fd = fd;
> +    struct kvm_ioeventfd iofd = {
> +        .datamatch = datamatch ? adjust_ioeventfd_endianness(val, size) : 0,
> +        .addr = addr,
> +        .len = size,
> +        .flags = 0,
> +        .fd = fd,
> +    };

Hm.  So, what's the difference?  The same fields are assigned the same
values, why in first case we have some uninitialized data and in second
case everything is initialized?  Does struct initializer zero-fills all
other places (alignments, missing fields etc) ?  If yes, there's no need
to assign zero to flags, btw ;)

Thanks,

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH] kvm: Silence warning from valgrind

[Thomas Huth]

On Wed, 29 Apr 2015 09:34:51 +0300
Michael Tokarev <mjt@tls.msk.ru> wrote:

> 27.04.2015 19:59, Thomas Huth wrote:
> > valgrind complains here about uninitialized bytes with the following message:
> > 
> > ==17814== Syscall param ioctl(generic) points to uninitialised byte(s)
> > ==17814==    at 0x466A780: ioctl (in /usr/lib64/power8/libc-2.17.so)
> > ==17814==    by 0x100735B7: kvm_vm_ioctl (kvm-all.c:1920)
> > ==17814==    by 0x10074583: kvm_set_ioeventfd_mmio (kvm-all.c:574)
> > 
> > Let's fix it by using a proper struct initializer in kvm_set_ioeventfd_mmio().
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  kvm-all.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/kvm-all.c b/kvm-all.c
> > index dd44f8c..077b0ed 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -552,13 +552,13 @@ static int kvm_set_ioeventfd_mmio(int fd, hwaddr addr, uint32_t val,
> >                                    bool assign, uint32_t size, bool datamatch)
> >  {
> >      int ret;
> > -    struct kvm_ioeventfd iofd;
> > -
> > -    iofd.datamatch = datamatch ? adjust_ioeventfd_endianness(val, size) : 0;
> > -    iofd.addr = addr;
> > -    iofd.len = size;
> > -    iofd.flags = 0;
> > -    iofd.fd = fd;
> > +    struct kvm_ioeventfd iofd = {
> > +        .datamatch = datamatch ? adjust_ioeventfd_endianness(val, size) : 0,
> > +        .addr = addr,
> > +        .len = size,
> > +        .flags = 0,
> > +        .fd = fd,
> > +    };
> 
> Hm.  So, what's the difference?  The same fields are assigned the same
> values, why in first case we have some uninitialized data and in second
> case everything is initialized?  Does struct initializer zero-fills all
> other places (alignments, missing fields etc) ?

Right, the struct initializer fills the remaining fields with zeros.

> If yes, there's no need to assign zero to flags, btw ;)

True. Shall I sent a patch without that line?

 Thomas

Re: [Qemu-devel] [PATCH] kvm: Silence warning from valgrind

[Michael Tokarev]

29.04.2015 09:43, Thomas Huth wrote:
> On Wed, 29 Apr 2015 09:34:51 +0300
> Michael Tokarev <mjt@tls.msk.ru> wrote:
[]
>> Hm.  So, what's the difference?  The same fields are assigned the same
>> values, why in first case we have some uninitialized data and in second
>> case everything is initialized?  Does struct initializer zero-fills all
>> other places (alignments, missing fields etc) ?
> 
> Right, the struct initializer fills the remaining fields with zeros.

Okay, point taken.  I just didn't know.

>> If yes, there's no need to assign zero to flags, btw ;)
> 
> True. Shall I sent a patch without that line?

I think it is better for readability to have it in.

Applied to -trivial, thank you!

/mjt