Oops, forgot to cc virt-tools-list earlier...
----- Original Message -----
> From: "Christophe Fergeau" <cfergeau@xxxxxxxxxx>
> To: "Jonathon Jongsma" <jjongsma@xxxxxxxxxx>
> Sent: Wednesday, October 30, 2013 5:27:18 AM
> Subject: Re: [virt-viewer 3/4] Reshow connection dialog on errors
>
> Hey,
>
> Don't know if you excluded the mailing list on purpose?
>
> On Tue, Oct 29, 2013 at 12:44:57PM -0400, Jonathon Jongsma wrote:
> > Can this be abstracted out into a separate function or something instead of
> > jumping around with gotos?
> >
> > e.g.
> >
> > do {
> > ret = remote_viewer_start_internal(self, guri);
> > } while (!ret && priv->open_recent_dialog)
> >
> > Or something of that sort? It's up to you, but I generally find that
> > functions with more than a single goto become more cumbersome to maintain.
>
> Yeah, I agree with you, though in this case it's actually quite awkward, as
> the internal function would basically need to be tristate, everything was
> OK, there was an error, or the user cancelled. Maybe there's an easier way,
> but the diff I ended up was much bigger than that one /o\
Yes, understood. I didn't actually investigate how feasible it was to change, so if it's not simple, go ahead with the current approach.
jonathon
>
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index 3b7b804..2454b2b 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -942,16 +942,22 @@ connect_dialog(gchar **uri)
> return retval;
> }
>
> -static gboolean
> -remote_viewer_start(VirtViewerApp *app)
> +enum RemoteViewerStartStatus {
> + START_OK,
> + START_CANCELLED,
> + START_ERROR
> +};
> +
> +static enum RemoteViewerStartStatus
> +remote_viewer_start_internal(VirtViewerApp *app)
> {
> - g_return_val_if_fail(REMOTE_VIEWER_IS(app), FALSE);
> + g_return_val_if_fail(REMOTE_VIEWER_IS(app), START_ERROR);
>
> RemoteViewer *self = REMOTE_VIEWER(app);
> RemoteViewerPrivate *priv = self->priv;
> GFile *file = NULL;
> VirtViewerFile *vvfile = NULL;
> - gboolean ret = FALSE;
> + enum RemoteViewerStartStatus ret = START_ERROR;
> gchar *guri = NULL;
> gchar *type = NULL;
> GError *error = NULL;
> @@ -981,12 +987,12 @@ remote_viewer_start(VirtViewerApp *app)
> retry_dialog:
> if (priv->open_recent_dialog) {
> if (connect_dialog(&guri) != 0)
> - return FALSE;
> + return START_CANCELLED;
> g_object_set(app, "guri", guri, NULL);
> } else
> g_object_get(app, "guri", &guri, NULL);
>
> - g_return_val_if_fail(guri != NULL, FALSE);
> + g_return_val_if_fail(guri != NULL, START_ERROR);
>
> DEBUG_LOG("Opening display to %s", guri);
> if ((virt_viewer_app_get_title(app) == NULL) || priv->default_title)
> {
> @@ -1039,7 +1045,11 @@ retry_dialog:
> }
> #endif
>
> - ret = VIRT_VIEWER_APP_CLASS(remote_viewer_parent_class)->start(app);
> + if (VIRT_VIEWER_APP_CLASS(remote_viewer_parent_class)->start(app)) {
> + ret = START_OK;
> + } else {
> + ret = START_ERROR;
> + }
>
> cleanup:
> g_clear_object(&file);
> @@ -1054,6 +1064,20 @@ cleanup:
> return ret;
> }
>
> +static gboolean
> +remote_viewer_start(VirtViewerApp *app)
> +{
> + RemoteViewer *self = REMOTE_VIEWER(app);
> + RemoteViewerPrivate *priv = self->priv;
> + enum RemoteViewerStartStatus ret;
> +
> + do {
> + ret = remote_viewer_start_internal(app);
> + } while ((ret == START_ERROR) && priv->open_recent_dialog);
> +
> + return (ret == START_OK);
> +}
> +
> /*
> * Local variables:
> * c-indent-level: 4
>
>
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
