[virt-tools-list] [virt-viewer 7/7] Create foreign menu from .vv file information

Jonathon Jongsma jjongsma at redhat.com
Thu Apr 17 15:25:30 UTC 2014


No, the leak doesn't really worry me. which is why I said the leak isn't a big deal ;)  I'm just thinking ahead to the future where some code might want to access the menu (for some reason) and we'd have to redesign things to make this possible (presumably via the app object). But maybe we'll never need access to this object, I don't know.  



----- Original Message -----
> From: "Marc-André Lureau" <mlureau at redhat.com>
> To: "Jonathon Jongsma" <jjongsma at redhat.com>
> Cc: "Christophe Fergeau" <cfergeau at redhat.com>, virt-tools-list at redhat.com
> Sent: Thursday, April 17, 2014 10:18:52 AM
> Subject: Re: [virt-tools-list] [virt-viewer 7/7] Create foreign menu from .vv	file information
> 
> 
> 
> ----- Original Message -----
> > ACK, but a general comment/question:
> > 
> > At the moment, the menu object is kind of floating and not owned by
> > anybody.
> > This means that it is leaked, which isn't a big deal since it's just a
> > single object that lives the whole life of the application. But it also
> > means there's no way to reference it outside of a signal handler. It might
> > be cleaner to have it owned by the app object? But maybe that will create
> > too much of an ifdef mess...
> > 
> > 
> 
> well, if that's only the leak that worries you, you could use
> g_object_set_data_full(app, ..) ?
> 
> > ----- Original Message -----
> > > From: "Christophe Fergeau" <cfergeau at redhat.com>
> > > To: virt-tools-list at redhat.com
> > > Sent: Wednesday, April 16, 2014 11:59:54 AM
> > > Subject: [virt-tools-list] [virt-viewer 7/7] Create foreign menu from .vv
> > > 	file information
> > > 
> > > When the .vv file has an [ovirt] section, we should try to create a
> > > foreign
> > > menu out of it. This will allow remote-viewer to offer a menu to change
> > > the
> > > currenty inserted cdrom.
> > > ---
> > >  src/virt-viewer-file.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > > 
> > > diff --git a/src/virt-viewer-file.c b/src/virt-viewer-file.c
> > > index 640b664..396df93 100644
> > > --- a/src/virt-viewer-file.c
> > > +++ b/src/virt-viewer-file.c
> > > @@ -24,6 +24,10 @@
> > >  #include <glib/gi18n.h>
> > >  #include <glib/gstdio.h>
> > >  
> > > +#ifdef HAVE_OVIRT
> > > +#include "ovirt-foreign-menu.h"
> > > +#endif
> > > +
> > >  #include "virt-viewer-util.h"
> > >  #include "virt-viewer-file.h"
> > >  
> > > @@ -749,6 +753,17 @@ virt_viewer_file_fill_app(VirtViewerFile* self,
> > > VirtViewerApp *app, GError **err
> > >          g_object_set(G_OBJECT(app), "fullscreen",
> > >              virt_viewer_file_get_fullscreen(self), NULL);
> > >  
> > > +#ifdef HAVE_OVIRT
> > > +    {
> > > +        OvirtForeignMenu *menu;
> > > +
> > > +        if (g_key_file_has_group(self->priv->keyfile, OVIRT_GROUP)) {
> > > +            menu = ovirt_foreign_menu_new_from_file(self);
> > > +            ovirt_foreign_menu_start(menu);
> > > +        }
> > > +    }
> > > +#endif
> > > +
> > >      return TRUE;
> > >  }
> > >  
> > > --
> > > 1.9.0
> > > 
> > > _______________________________________________
> > > virt-tools-list mailing list
> > > virt-tools-list at redhat.com
> > > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > > 
> > 
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > 
> 




More information about the virt-tools-list mailing list