[virt-tools-list] [PATCH virt-viewer] Fix segmentation fault on zoom

[Pavel Grunt]

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 at redhat.com> wrote: 
> > Charles.
> > 
> > On Tue, Jun 7, 2016 at 9:51 PM, Charles Arnold <carnold at suse.com> 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 at suse.com>
> > > 
> > > 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 at redhat.com
> > > 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 at 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list