[Qemu-devel] Re: [PATCH] Wrapper around dpy_copy to fix segfault with -vnc option

[andrzej zaborowski]

2008/9/23 Jan Niehusmann <address@hidden>:
> On Mon, Sep 22, 2008 at 11:15:04PM +0200, andrzej zaborowski wrote:
>> Yes, I don't think hw/ code should be concerned with what console is
>> active.  Logically the dpy_ functions should take the pointer returned
>> from graphic_console_init() as first parameter.
>>
>> Please also check the code is formatted consistently with qemu.
>>
>> I didn't receive Jan's message but the check seems to not be enough
>> because there can be multiple graphical consoles with different sizes
>> - if I'm guessing correctly what this patch tries to fix.
>
> Based on these comments I had another look at the code. If there can
> be multiple graphical consoles, the only sensible test is 'console
> == active_console' where console must be provided by the caller. So,
> indeed, a pointer to the console must be provided instead of a pointer
> to the DisplayState.
>
> To make function names consistent, I called the function qemu_console_copy
> in analogy to qemu_console_resize (which is a similar wrapper around
> dpy_resize).

I committed the patch slightly modified because I found this still
doesn't account for all the cases.  Imagine ds->dpy_copy is not set,
the call does nothing and the screen is not updated until fully
invalidated.  We need to either implement a generic dpy_copy in
console.c or have a fallback in hw/cirrus_vga.c depending on which is
faster.

Also note that till now hw/ files could only call into the ds->dpy_
functions from inside their own vga_hw_update callback, this
guaranteed some consistency.  The use of dpy_copy inside cirrus_vga.c
changed this which is the source of these bugs.  I hadn't noticed when
this happened.

Regards