Re: [Qemu-devel] [PATCH] Unbreak -no-quit for GTK when SDL is disabled

[Peter Wu]

On Monday 10 June 2013 16:53:06 Anthony Liguori wrote:
> Peter Wu <address@hidden> writes:
> > When QEMU is built without SDL support, -no-quit would print an error
> > message that SDL is disabled. Since GTK also supports -no-quit, enable
> > the option when GTK or SDL is enabled at compile time.
> > 
> > While at it, do not create the no_quit variable when it is not used.
> > 
> > Signed-off-by: Peter Wu <address@hidden>
> 
> Any harm in just making -no-quit unconditional?

No harm, there are just 4 bytes more memory wasted and some code, not a big 
deal.

> It's a nop for VNC and presumably for spice too but it's not incorrect
> AFAICT.
> 
> Would be nice to drop #ifdefs if we can.

The readability should increase by dropping it. By the way, this integer 
should really be a bool.

As shortly discussed on IRC, a better approach is testing for the sanity of 
certain usages instead of ifdef'ing stuff. Having SDL compiled in and then 
using VNC still does not make -alt-grab more valid.

Let's drop this patch, I will submit a new one which checks the option at run-
time.

Regards,
Peter

> Regards,
> 
> Anthony Liguori
> 
> > ---
> > 
> >  include/sysemu/sysemu.h |  2 ++
> >  vl.c                    | 13 +++++++++----
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index 2fb71af..b9b8e52 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -117,7 +117,9 @@ extern int smp_cpus;
> > 
> >  extern int max_cpus;
> >  extern int cursor_hide;
> >  extern int graphic_rotate;
> > 
> > +#if defined(CONFIG_SDL) || defined(CONFIG_GTK)
> > 
> >  extern int no_quit;
> > 
> > +#endif
> > 
> >  extern int no_shutdown;
> >  extern int semihosting_enabled;
> >  extern int old_param;
> > 
> > diff --git a/vl.c b/vl.c
> > index cfd2d3e..74ab050 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -202,7 +202,9 @@ static int full_screen = 0;
> > 
> >  #ifdef CONFIG_SDL
> >  static int no_frame = 0;
> >  #endif
> > 
> > +#if defined(CONFIG_SDL) || defined(CONFIG_GTK)
> > 
> >  int no_quit = 0;
> > 
> > +#endif
> > 
> >  CharDriverState *serial_hds[MAX_SERIAL_PORTS];
> >  CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
> >  CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
> > 
> > @@ -3523,6 +3525,13 @@ int main(int argc, char **argv, char **envp)
> > 
> >              case QEMU_OPTION_full_screen:
> >                  full_screen = 1;
> >                  break;
> > 
> > +            case QEMU_OPTION_no_quit:
> > +#if defined(CONFIG_SDL) || defined(CONFIG_GTK)
> > +                no_quit = 1;
> > +#else
> > +                fprintf(stderr, "SDL and GTK support are disabled\n");
> > +#endif
> > +                break;
> > 
> >  #ifdef CONFIG_SDL
> >  
> >              case QEMU_OPTION_no_frame:
> >                  no_frame = 1;
> > 
> > @@ -3533,9 +3542,6 @@ int main(int argc, char **argv, char **envp)
> > 
> >              case QEMU_OPTION_ctrl_grab:
> >                  ctrl_grab = 1;
> >                  break;
> > 
> > -            case QEMU_OPTION_no_quit:
> > -                no_quit = 1;
> > -                break;
> > 
> >              case QEMU_OPTION_sdl:
> >                  display_type = DT_SDL;
> >                  break;
> > 
> > @@ -3543,7 +3549,6 @@ int main(int argc, char **argv, char **envp)
> > 
> >              case QEMU_OPTION_no_frame:
> >              case QEMU_OPTION_alt_grab:
> > 
> >              case QEMU_OPTION_ctrl_grab:
> > -            case QEMU_OPTION_no_quit:
> >              case QEMU_OPTION_sdl:
> >                  fprintf(stderr, "SDL support is disabled\n");
> >                  exit(1);