[virt-tools-list] [PATCH virt-viewer 7/7] remote-viewer: learn to connect from file

[Christophe Fergeau]

On Tue, Nov 27, 2012 at 01:57:30PM +0100, Marc-André Lureau wrote:
> On Tue, Nov 27, 2012 at 10:55 AM, Christophe Fergeau <cfergeau at redhat.com>wrote:
> 
> > Hey,
> >
> > On Fri, Nov 23, 2012 at 01:41:12PM +0100, Marc-André Lureau wrote:
> > > ---
> > >  src/remote-viewer.c | 25 +++++++++++++++++++++++--
> > >  1 file changed, 23 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> > > index 72b1ca8..553f251 100644
> > > --- a/src/remote-viewer.c
> > > +++ b/src/remote-viewer.c
> > > @@ -35,6 +35,7 @@
> > >  #include "virt-viewer-session-spice.h"
> > >  #endif
> > >  #include "virt-viewer-app.h"
> > > +#include "virt-viewer-file.h"
> > >  #include "remote-viewer.h"
> > >
> > >  #ifndef G_VALUE_INIT /* see bug
> > https://bugzilla.gnome.org/show_bug.cgi?id=654793 */
> > > @@ -626,9 +627,13 @@ remote_viewer_start(VirtViewerApp *app)
> > >      RemoteViewer *self = REMOTE_VIEWER(app);
> > >      RemoteViewerPrivate *priv = self->priv;
> > >  #endif
> > > +    GFile *file = NULL;
> > > +    VirtViewerFile *vvfile = NULL;
> > >      gboolean ret = FALSE;
> > >      gchar *guri = NULL;
> > >      gchar *type = NULL;
> > > +    gchar *path = NULL;
> > > +    GError *error = NULL;
> > >
> > >  #if HAVE_SPICE_GTK
> > >      g_signal_connect(app, "notify", G_CALLBACK(app_notified), self);
> > > @@ -659,7 +664,17 @@ remote_viewer_start(VirtViewerApp *app)
> > >          if (virt_viewer_app_get_title(app) == NULL)
> > >              virt_viewer_app_set_title(app, guri);
> > >
> > > -        if (virt_viewer_util_extract_host(guri, &type, NULL, NULL,
> > NULL, NULL) < 0 || type == NULL) {
> > > +        file = g_file_new_for_commandline_arg(guri);
> > > +        if (g_file_query_exists(file, NULL)) {
> > > +            path = g_file_get_path(file);
> > > +            vvfile = virt_viewer_file_new(path, &error);
> >
> > I'd make 'error' and 'path' local to that block, the function is a bit big,
> > so that makes less things to mentally track for the whole function. Looks
> > good apart from this.
> >
> 
> ok, I'll try to split it with a seperate function, since using local
> variable make the cleanup handling more complicated.

Oh, don't bother unless you agree this makes things better. It didn't seem to me
that moving path and error in this inner block would make the code more
complicated (ie just move the path/error cleanup to the inner block right
after last use), but it seems I missed something ;)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20121127/76ca0d34/attachment.sig>