[virt-tools-list] [PATCH 1/3]virt-viewer: Quit virt-viewer app when cancel button is clicked

[Guan Nan Ren]

Hi Marc-André, 

----- Original Message -----
From: "Marc-André Lureau" <marcandre.lureau at gmail.com>
To: "Guannan Ren" <gren at redhat.com>
Cc: virt-tools-list at redhat.com
Sent: Monday, January 16, 2012 9:31:14 PM
Subject: Re: [virt-tools-list] [PATCH 1/3]virt-viewer: Quit virt-viewer app when cancel button is clicked

Hi

On Sun, Jan 15, 2012 at 7:29 AM, Guannan Ren <gren at redhat.com> wrote:
> When using virt-viewer for a guest with spice as its graphic, it will
> ask for the authentication in the case of password setting. After clicking
> cancell, it gives a message dialog:
>
>        "Unable to authenticate with remote desktop server at localhost:5900:
>         Unable to collect credentials.Retry connection again?"
>
> That is not expected, it should exit instead like what vnc did.

It's usually nicer to describe what the patch does. Describing the
patch series is better in a seperate summary.

got it, thanks.

> ---
>  src/virt-viewer-auth.c |   17 ++++++++++++++---
>  1 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/src/virt-viewer-auth.c b/src/virt-viewer-auth.c
> index d6c0300..e10690b 100644
> --- a/src/virt-viewer-auth.c
> +++ b/src/virt-viewer-auth.c
> @@ -46,6 +46,7 @@ virt_viewer_auth_collect_credentials(const char *type,
>        GtkWidget *promptPassword;
>        GtkWidget *labelMessage;
>        int response;
> +       int ret;
>        char *message;
>
>        dialog = GTK_WIDGET(gtk_builder_get_object(creds, "auth"));
> @@ -79,16 +80,26 @@ virt_viewer_auth_collect_credentials(const char *type,
>        response = gtk_dialog_run(GTK_DIALOG(dialog));
>        gtk_widget_hide(dialog);
>
> -       if (response == GTK_RESPONSE_OK) {
> +       switch(response) {
> +       case GTK_RESPONSE_OK:
>                if (username)
>                        *username = g_strdup(gtk_entry_get_text(GTK_ENTRY(credUsername)));
>                if (password)
>                        *password = g_strdup(gtk_entry_get_text(GTK_ENTRY(credPassword)));
> +               ret = 1;
> +               break;
> +
> +       case GTK_RESPONSE_CANCEL:
> +               ret = -1;
> +               break;
> +
> +       default:
> +               ret = 0;
>        }
>
>        gtk_widget_destroy(GTK_WIDGET(dialog));
>
> -       return response == GTK_RESPONSE_OK ? 0 : -1;
> +       return ret;
>  }
>
>  #ifdef HAVE_GTK_VNC
> @@ -126,7 +137,7 @@ virt_viewer_auth_vnc_credentials(GtkWidget *vnc,
>                                                               wantUsername ? &username : NULL,
>                                                               wantPassword ? &password : NULL);
>
> -               if (ret < 0) {
> +               if (ret <= 0) {
>                        vnc_display_close(VNC_DISPLAY(vnc));
>                        goto cleanup;
>                }

Can you explain why you changed a dialog that used to return 0 Ok or
-1 Cancel value to 1, 0, -1 values? what 0 means? Would be worth
having an enum perhaps.

if 0 means fail too now, then the Spice condition needs to be changed
here as well in src/virt-viewer-session-spice.c. So each patch can be
applied and tested seperatly without regression.

thanks

Yep, the 2 return values is enough, 0 is redundant.
I will keep it unchanged to this function as well as
the vnc part in the next patch series.

thanks.

-- 
Marc-André Lureau