Hey, On Tue, Sep 23, 2014 at 04:57:41PM -0500, Jonathon Jongsma wrote: > When using the fullscreen display mapping configuration file, extra > monitors could end up enabled by mistake. This was because > virt_viewer_app_get_initial_monitor_for_display would end up returning > Nmonitor = Ndisplay when the display map hash lookup failed. In > reality, when a display map is specified, but the hash lookup fails, > the display should not be enabled. This function now returns -1 to > distinguish this case, and the display is not enabled when this value is > returned. > --- > > The implementation was apparently incomplete. This patch attempts to fix an > issue that was found during testing: > see https://bugzilla.redhat.com/show_bug.cgi?id=1129477#c9 > > src/virt-viewer-app.c | 22 ++++++++++++---------- > src/virt-viewer-session-spice.c | 3 +++ > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c > index a890bf6..12d01eb 100644 > --- a/src/virt-viewer-app.c > +++ b/src/virt-viewer-app.c > @@ -314,8 +314,11 @@ gint virt_viewer_app_get_initial_monitor_for_display(VirtViewerApp* self, gint d > > if (self->priv->initial_display_map) { > gpointer value = NULL; > - if (g_hash_table_lookup_extended(self->priv->initial_display_map, GINT_TO_POINTER(display), NULL, &value)) > + if (g_hash_table_lookup_extended(self->priv->initial_display_map, GINT_TO_POINTER(display), NULL, &value)) { > monitor = GPOINTER_TO_INT(value); > + } else { > + monitor = -1; > + } > } My understanding is that self->priv->initial_display_map is only set when a display configuration file is in used? > > return monitor; > @@ -326,13 +329,14 @@ app_window_try_fullscreen(VirtViewerApp *self G_GNUC_UNUSED, > VirtViewerWindow *win, gint nth) > { > GdkScreen *screen = gdk_screen_get_default(); > - > - if (nth >= gdk_screen_get_n_monitors(screen)) { > - g_debug("skipping display %d", nth); > + gint monitor = virt_viewer_app_get_initial_monitor_for_display(self, nth); > + if (monitor == -1 || monitor >= gdk_screen_get_n_monitors(screen)) { > + g_debug("skipping fullscreen for display %d", nth); > + virt_viewer_window_hide(win); > return; > } > > - virt_viewer_window_enter_fullscreen(win, nth); > + virt_viewer_window_enter_fullscreen(win, monitor); > } > > > @@ -449,8 +453,7 @@ void virt_viewer_app_set_uuid_string(VirtViewerApp *self, const gchar *uuid_stri > > g_hash_table_iter_init(&iter, self->priv->windows); > while (g_hash_table_iter_next(&iter, NULL, &value)) { > - gint monitor = virt_viewer_app_get_initial_monitor_for_display(self, i); > - app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value), monitor); > + app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value), i); > i++; > } For what it's worth, the move of virt_viewer_app_get_initial_monitor_for_display() in app_window_try_fullscreen() could have been done in a preparatory commit. ACK if my understanding about initial_display_map is correct. Christophe
Attachment:
pgp83cAnLVzWR.pgp
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
