[PATCHv2,1/3] ui/spice-display.c: add missing initialization for valgrind

We can't initialize QXLDevSurfaceCreate field by field because it has a
pa hole, and so 4 bytes remain uninitialized when building on x86-64, so
just memset.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 ui/spice-display.c |    2 ++
 1 file changed, 2 insertions(+)

On 05/22/12 17:29, Alon Levy wrote:
> We can't initialize QXLDevSurfaceCreate field by field because it has a
> pa hole, and so 4 bytes remain uninitialized when building on x86-64, so
> just memset.

So you get valgrind warnings for the hole?  why?  nobody should ever
access the hole, so the missing initialization should not hurt in theory ...

> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  ui/spice-display.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 5418eb3..3e8f0b3 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -244,6 +244,8 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
>  {
>      QXLDevSurfaceCreate surface;
>  
> +    memset(&surface, 0, sizeof(surface));
> +
>      dprint(1, "%s: %dx%d\n", __FUNCTION__,
>             ds_get_width(ssd->ds), ds_get_height(ssd->ds));
>

On Wed, May 23, 2012 at 08:59:22PM +0200, Gerd Hoffmann wrote:
> On 05/22/12 17:29, Alon Levy wrote:
> > We can't initialize QXLDevSurfaceCreate field by field because it has a
> > pa hole, and so 4 bytes remain uninitialized when building on x86-64, so
> > just memset.
> 
> So you get valgrind warnings for the hole?  why?  nobody should ever
> access the hole, so the missing initialization should not hurt in theory ...

Because we allocate this struct on the stack and then copy it over an fd
to spice, through the dispatcher pipe.

echo quit | valgrind --log-file=/tmp/valgrind.log
--leak-check=full ./x86_64-softmmu/qemu-system-x86_64 -monitor stdio
-spice port=1234 -vga qxl

==2677== Syscall param write(buf) points to uninitialised byte(s)
==2677==    at 0x659504D: ??? (syscall-template.S:82)
==2677==    by 0x91BCD54: write_safe (dispatcher.c:103)
==2677==    by 0x91BD168: dispatcher_send_message (dispatcher.c:182)
==2677==    by 0x91BEC24: red_dispatcher_create_primary_surface_sync (red_dispatcher.c:489)
==2677==    by 0x91BECAD: red_dispatcher_create_primary_surface (red_dispatcher.c:502)
==2677==    by 0x91BED03: qxl_worker_create_primary_surface (red_dispatcher.c:509)
==2677==    by 0x3403CC: qemu_spice_create_primary_surface (spice-display.c:106)
==2677==    by 0x340BAB: qemu_spice_create_host_primary (spice-display.c:260)
==2677==    by 0x40EC50: qxl_enter_vga_mode (qxl.c:931)
==2677==    by 0x40EFCA: qxl_soft_reset (qxl.c:982)
==2677==    by 0x40F088: qxl_hard_reset (qxl.c:1004)
==2677==    by 0x40F0DA: qxl_reset_handler (qxl.c:1011)
==2677==  Address 0x7feffef98 is on thread 1's stack
> 
> > Signed-off-by: Alon Levy <alevy@redhat.com>
> > ---
> >  ui/spice-display.c |    2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > index 5418eb3..3e8f0b3 100644
> > --- a/ui/spice-display.c
> > +++ b/ui/spice-display.c
> > @@ -244,6 +244,8 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
> >  {
> >      QXLDevSurfaceCreate surface;
> >  
> > +    memset(&surface, 0, sizeof(surface));
> > +
> >      dprint(1, "%s: %dx%d\n", __FUNCTION__,
> >             ds_get_width(ssd->ds), ds_get_height(ssd->ds));
> >  
>

On 05/24/12 08:43, Alon Levy wrote:
> On Wed, May 23, 2012 at 08:59:22PM +0200, Gerd Hoffmann wrote:
>> On 05/22/12 17:29, Alon Levy wrote:
>>> We can't initialize QXLDevSurfaceCreate field by field because it has a
>>> pa hole, and so 4 bytes remain uninitialized when building on x86-64, so
>>> just memset.
>>
>> So you get valgrind warnings for the hole?  why?  nobody should ever
>> access the hole, so the missing initialization should not hurt in theory ...
> 
> Because we allocate this struct on the stack and then copy it over an fd
> to spice, through the dispatcher pipe.

Ah, yea, copying will make valgrind complain of course ...

I'll go queue it up.

cheers,
  Gerd

Hi,

> I'll go queue it up.

Ahem, doesn't apply cleanly.  Guess this is just a patch ordering issue
though.  Can you send all your pending qxl bits as single patch series
please, so I don't have to play trial-and-error to figure the correct order?

thanks,
  Gerd