[virt-tools-list] [PATCH virt-viewer 4/4] Exit normally when canceling dialog

[Fabiano Fidêncio]

On Tue, Mar 17, 2015 at 10:08 AM, Pavel Grunt <pgrunt at redhat.com> wrote:
> This applies for:
>  session authentication dialog
>  libvirt authentication dialog (e.g. virt-viewer --attach guest)
>  oVirt authentication dialog (e.g. remote-viewer ovirt://host/guest)
>  'vm choose' dialog when connecting specifying the vm name
>
> Related to https://bugzilla.redhat.com/show_bug.cgi?id=1201604
> ---
>  src/remote-viewer.c             | 4 ++++
>  src/virt-viewer-session-spice.c | 2 ++
>  src/virt-viewer-session-vnc.c   | 1 +
>  src/virt-viewer.c               | 7 ++++++-
>  4 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index f9f4f92..c6c151b 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -876,6 +876,7 @@ create_ovirt_session(VirtViewerApp *app, const char *uri, GError **err)
>
>      api = ovirt_proxy_fetch_api(proxy, &error);
>      if (authenticate_info.dialog_cancelled) {
> +        virt_viewer_app_set_user_cancelled(app, TRUE);
>          g_clear_error(&error);
>          goto error;
>      }
> @@ -897,6 +898,9 @@ create_ovirt_session(VirtViewerApp *app, const char *uri, GError **err)
>                         vms,
>                         &error);
>          if (vm == NULL) {
> +            if (error == NULL) {
> +                virt_viewer_app_set_user_cancelled(app, TRUE);
> +            }
>              goto error;
>          }
>      }
> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> index 5eb7234..93906ae 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -562,6 +562,7 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel G_GNUC_UNUSED
>                                                     username_required ? &user : NULL,
>                                                     &password);
>          if (!ret) {
> +            virt_viewer_app_set_user_cancelled(virt_viewer_session_get_app(session), TRUE);
>              g_signal_emit_by_name(session, "session-cancelled");
>          } else {
>              gboolean openfd;
> @@ -593,6 +594,7 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel G_GNUC_UNUSED
>                                                         "proxy", NULL,
>                                                         &user, &password);
>              if (!ret) {
> +                virt_viewer_app_set_user_cancelled(virt_viewer_session_get_app(session), TRUE);
>                  g_signal_emit_by_name(session, "session-cancelled");
>              } else {
>                  spice_uri_set_user(proxy, user);
> diff --git a/src/virt-viewer-session-vnc.c b/src/virt-viewer-session-vnc.c
> index 5a2dd86..68a6719 100644
> --- a/src/virt-viewer-session-vnc.c
> +++ b/src/virt-viewer-session-vnc.c
> @@ -304,6 +304,7 @@ virt_viewer_session_vnc_auth_credential(GtkWidget *src G_GNUC_UNUSED,
>                                                              wantPassword ? &password : NULL);
>
>          if (!ret) {
> +            virt_viewer_app_set_user_cancelled(virt_viewer_session_get_app(session), TRUE);
>              vnc_display_close(self->priv->vnc);
>              goto cleanup;
>          }
> diff --git a/src/virt-viewer.c b/src/virt-viewer.c
> index acad6c6..65ec4e3 100644
> --- a/src/virt-viewer.c
> +++ b/src/virt-viewer.c
> @@ -730,7 +730,10 @@ virt_viewer_initial_connect(VirtViewerApp *app, GError **error)
>                              &priv->domkey,
>                              priv->conn,
>                              &err);
> -            if (dom == NULL && err != NULL) {
> +            if (dom == NULL) {
> +                if (err == NULL) {
> +                    virt_viewer_app_set_user_cancelled(app, TRUE);
> +                }

Here you changed a bit the logic and I'm not sure if it was intentional or not.
I meant, the only way to go to clean up was: dom == NULL *and* err !=
NULL. After your changes, dom == NULL is enough to go to clean up.
Is it intentional? Are you missing an else for the "err == NULL" if?

>                  goto cleanup;
>              }
>          }
> @@ -910,6 +913,8 @@ virt_viewer_connect(VirtViewerApp *app)
>              virt_viewer_app_simple_message_dialog(app, error_message);
>
>              g_free(error_message);
> +        } else {
> +            virt_viewer_app_set_user_cancelled(app, TRUE);
>          }
>
>          return -1;
> --
> 2.3.2
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list

A more general comment ... I'd set
virt_viewer_app_set_user_cancelled() in all possible situations, being
TRUE or FALSE and not only when it is TRUE.
Seems a bit more clear, at least for me :-)

-- 
Fabiano Fidêncio