Re: [Qemu-devel] [PATCH] ui: sdl2: Fix crash with -nodefaults -sdl

[Cole Robinson]

On 06/01/2016 02:38 AM, Gerd Hoffmann wrote:
> On Di, 2016-05-31 at 16:56 -0400, Cole Robinson wrote:
>> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -sdl
>> Segmentation fault (core dumped)
>>
>> 0  0x559631af in sdl_display_init (ds=, 
>> full_screen=0, no_frame=) at ui/sdl2.c:822
>> 1  0x556c8a9a in main (argc=, argv=, 
>> envp=) at vl.c:4527
>>
>> Setting the window icon assumes there's always an SDL output window
>> available, which isn't the case with when there's no video device,
>> like via -nodefaults. So don't try to set a window icon when we don't
>> have any outputs.
> 
> Hmm, I guess we can skip pretty much all of the init in case there are
> no outputs:
> 
> @@ -794,6 +794,9 @@ void sdl_display_init(DisplayState *ds, int
> full_screen, int no_frame)
>  }
>  }
>  sdl2_num_outputs = i;
> +if (sdl2_num_outputs == 0) {
> +return;
> +}
>  sdl2_console = g_new0(struct sdl2_console, sdl2_num_outputs);
>  for (i = 0; i < sdl2_num_outputs; i++) {
>  QemuConsole *con = qemu_console_lookup_by_index(i);
> 
> 
> Maybe even move up the loop counting the outputs, so we can skip the
> SDL_Init() call too.  We don't get a empty window then.
> 

Works for me, I figured there was a better approach to handle -sdl + no
display device. If you send a patch I'll test it

Thanks,
Cole

Re: [Qemu-devel] [PATCH] ui: sdl2: Fix crash with -nodefaults -sdl

[Gerd Hoffmann]

On Di, 2016-05-31 at 22:49 +0100, Peter Maydell wrote:
> On 31 May 2016 at 21:56, Cole Robinson  wrote:
> > $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -sdl
> > Segmentation fault (core dumped)
> >
> > 0  0x559631af in sdl_display_init (ds=, 
> > full_screen=0, no_frame=) at ui/sdl2.c:822
> > 1  0x556c8a9a in main (argc=, argv=, 
> > envp=) at vl.c:4527
> >
> > Setting the window icon assumes there's always an SDL output window
> > available, which isn't the case with when there's no video device,
> > like via -nodefaults. So don't try to set a window icon when we don't
> > have any outputs.
> 
> Presumably we also crash for boards like the arm 'virt'
> which just don't have a display device at all...

There are still the vc's for monitor and serial.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] ui: sdl2: Fix crash with -nodefaults -sdl

[Gerd Hoffmann]

On Di, 2016-05-31 at 16:56 -0400, Cole Robinson wrote:
> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -sdl
> Segmentation fault (core dumped)
> 
> 0  0x559631af in sdl_display_init (ds=, full_screen=0, 
> no_frame=) at ui/sdl2.c:822
> 1  0x556c8a9a in main (argc=, argv=, 
> envp=) at vl.c:4527
> 
> Setting the window icon assumes there's always an SDL output window
> available, which isn't the case with when there's no video device,
> like via -nodefaults. So don't try to set a window icon when we don't
> have any outputs.

Hmm, I guess we can skip pretty much all of the init in case there are
no outputs:

@@ -794,6 +794,9 @@ void sdl_display_init(DisplayState *ds, int
full_screen, int no_frame)
 }
 }
 sdl2_num_outputs = i;
+if (sdl2_num_outputs == 0) {
+return;
+}
 sdl2_console = g_new0(struct sdl2_console, sdl2_num_outputs);
 for (i = 0; i < sdl2_num_outputs; i++) {
 QemuConsole *con = qemu_console_lookup_by_index(i);


Maybe even move up the loop counting the outputs, so we can skip the
SDL_Init() call too.  We don't get a empty window then.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] ui: sdl2: Fix crash with -nodefaults -sdl

[Peter Maydell]

On 31 May 2016 at 21:56, Cole Robinson  wrote:
> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -sdl
> Segmentation fault (core dumped)
>
> 0  0x559631af in sdl_display_init (ds=, full_screen=0, 
> no_frame=) at ui/sdl2.c:822
> 1  0x556c8a9a in main (argc=, argv=, 
> envp=) at vl.c:4527
>
> Setting the window icon assumes there's always an SDL output window
> available, which isn't the case with when there's no video device,
> like via -nodefaults. So don't try to set a window icon when we don't
> have any outputs.

Presumably we also crash for boards like the arm 'virt'
which just don't have a display device at all...

thanks
-- PMM

[Qemu-devel] [PATCH] ui: sdl2: Fix crash with -nodefaults -sdl

[Cole Robinson]

$ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -sdl
Segmentation fault (core dumped)

0  0x559631af in sdl_display_init (ds=, full_screen=0, 
no_frame=) at ui/sdl2.c:822
1  0x556c8a9a in main (argc=, argv=, 
envp=) at vl.c:4527

Setting the window icon assumes there's always an SDL output window
available, which isn't the case with when there's no video device,
like via -nodefaults. So don't try to set a window icon when we don't
have any outputs.

https://bugzilla.redhat.com/show_bug.cgi?id=1340931
---
 ui/sdl2.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 909038f..d0e0a41 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -812,16 +812,18 @@ void sdl_display_init(DisplayState *ds, int full_screen, 
int no_frame)
 register_displaychangelistener(_console[i].dcl);
 }
 
-/* Load a 32x32x4 image. White pixels are transparent. */
-filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "qemu-icon.bmp");
-if (filename) {
-SDL_Surface *image = SDL_LoadBMP(filename);
-if (image) {
-uint32_t colorkey = SDL_MapRGB(image->format, 255, 255, 255);
-SDL_SetColorKey(image, SDL_TRUE, colorkey);
-SDL_SetWindowIcon(sdl2_console[0].real_window, image);
+if (sdl2_num_outputs) {
+/* Load a 32x32x4 image. White pixels are transparent. */
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "qemu-icon.bmp");
+if (filename) {
+SDL_Surface *image = SDL_LoadBMP(filename);
+if (image) {
+uint32_t colorkey = SDL_MapRGB(image->format, 255, 255, 255);
+SDL_SetColorKey(image, SDL_TRUE, colorkey);
+SDL_SetWindowIcon(sdl2_console[0].real_window, image);
+}
+g_free(filename);
 }
-g_free(filename);
 }
 
 if (full_screen) {
-- 
2.5.5