Hi
On Sun, Jan 15, 2012 at 7:29 AM, Guannan Ren <gren@xxxxxxxxxx> 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.
> ---
> 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
--
Marc-André Lureau
