[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 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

Attachment: pgpkO2IwjwID9.pgp
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]