Hi,
good catch indeed. In my opinion it should not be allowed to active these zoom
callbacks when display is not ready - I though it was working.
On Tue, 2016-06-07 at 15:03 -0600, Charles Arnold wrote:
> >
> > > > On 6/7/2016 at 02:38 PM, Fabiano Fidêncio <fidencio@xxxxxxxxxx> wrote:
> > Charles.
> >
> > On Tue, Jun 7, 2016 at 9:51 PM, Charles Arnold <carnold@xxxxxxxx> wrote:
> > > When virt-viewer is "Waiting for guest domain to start" and
> > > the Ctrl- or Ctrl+ keys are pressed to zoom the blank display
> > > virt-viewer will crash in virt_viewer_display_get_desktop_size
> > > because of a NULL display pointer. To reproduce start virt-viewer
> > > on a VM not running and zoom the display.
> >
> > Nice catch!
> >
> > >
> > > Signed-off-by: Charles Arnold <carnold@xxxxxxxx>
> > >
> > > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> > > index ef62d9a..939f5f8 100644
> > > --- a/src/virt-viewer-window.c
> > > +++ b/src/virt-viewer-window.c
> > > @@ -388,6 +388,9 @@ G_MODULE_EXPORT void
> > > virt_viewer_window_menu_view_zoom_out(GtkWidget *menu G_GNUC_UNUSED,
> > > VirtViewerWindow *self)
> > > {
> > > + if ( self->priv->display == NULL )
> > > + return;
> > > +
> > > virt_viewer_window_set_zoom_level(self,
> > >
> > virt_viewer_window_get_real_zoom_level(self) - ZOOM_STEP);
> > > }
> > > @@ -396,6 +399,9 @@ G_MODULE_EXPORT void
> > > virt_viewer_window_menu_view_zoom_in(GtkWidget *menu G_GNUC_UNUSED,
> > > VirtViewerWindow *self)
> > > {
> > > + if ( self->priv->display == NULL )
> > > + return;
> > > +
> > > virt_viewer_window_set_zoom_level(self,
> > >
> > virt_viewer_window_get_real_zoom_level(self) + ZOOM_STEP);
> > > }
> > >
> > >
> > >
> > > _______________________________________________
> > > virt-tools-list mailing list
> > > virt-tools-list@xxxxxxxxxx
> > > https://www.redhat.com/mailman/listinfo/virt-tools-list
> >
> > I'd prefer to have the check done on
> > virt_viewer_window_get_real_zoom_level(), like the example below. What
> > do you think? If you agree I'll do the change before pushing and keep
> > your ownership of the patch.
>
> No problem. That will also work.
>
> - Charles
>
> >
> > [ffidenci@cat virt-viewer]$ git diff
> > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> > index ef62d9a..38effab 100644
> > --- a/src/virt-viewer-window.c
> > +++ b/src/virt-viewer-window.c
> > @@ -378,6 +378,9 @@
> > virt_viewer_window_get_real_zoom_level(VirtViewerWindow *self)
> > GtkAllocation allocation;
> > guint width, height;
> >
> > + if (self->priv->display == NULL)
> > + return 0;
> > +
hmm, is 0 a good value for "real zoom"? It definitely should be protected, but I
would consider it as an programmer's error:
g_return_val_if_fail(self->priv->display != NULL, 100)
Regards,
Pavel
> > gtk_widget_get_allocation(GTK_WIDGET(self->priv->display),
> > &allocation);
> > virt_viewer_display_get_desktop_size(self->priv->display, &width,
> > &height);
> >
> > Best Regards,
> > --
> > Fabiano Fidêncio
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/virt-tools-list
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
