Re: [PATCH v2] ui/gtk: Allow user to select monitor number to display qemu in full screen through new gtk display option

[Gerd Hoffmann <kraxel@redhat.com>]

> > I don't think qemu has to worry much here, and trying to automatically
> > adapt to hot-plugged monitors might even have bad interactions with
> > whatever the display server is going to do.
> 
> I'm concerned there is a TOCTTOU issue:
> 
>     if (opts->u.gtk.has_monitor) {
>         int n_monitor;
> 1.      n_monitor = gdk_display_get_n_monitors(window_display);
> 
> 2.      if ((opts->u.gtk.monitor <= n_monitor) &&
>             (opts->u.gtk.monitor > 0)) {
>             GdkScreen *gdk_screen;
>             gdk_screen = gdk_display_get_default_screen(window_display);
> 3.          gtk_window_fullscreen_on_monitor(GTK_WINDOW(s->window), gdk_screen,
>                                              (opts->u.gtk.monitor - 1));
>         } else {
>             fprintf(stderr, "Invalid GTK monitor argument\n");
>         }
>     }
> 
> If monitors can go at any time, then the check 2. cannot ensure we pass
> a valid monitor number at 3.
> 
> I asked what happens when we pass an invalid monitor number.  I'm not
> sure I understand sweeaun's answer.
> 
> If what happens is sane, then why have check 2.?

gtk_window_fullscreen_on_monitor() seems to be a "best effort" offer and
it doesn't return error codes.  So catching possible user errors looks
useful to me.  The code should use error_report instead though (or maybe
warn_report given this is not fatal).  Reporting the valid range is
probably a good idea too.

take care,
  Gerd