[virt-tools-list] [virt-viewer PATCH] Show errors generated by connection dialog

Pavel Grunt pgrunt at redhat.com
Thu Feb 9 10:23:38 UTC 2017


Hello Christophe,

On Wed, 2017-02-08 at 17:48 +0100, Christophe de Dinechin wrote:
> When running 'remote-viewer' without arguments,
> and selecting a non-supported protocol, e.g. ssh://foo,
> the generated error was silently swallowed by the retry loop.
> This was introduced in 06b2c382468876a19600374bd59475e66d488af8.
> ---
>  src/remote-viewer.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index 13c6e7c..c9ef4bb 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -1196,6 +1196,9 @@ cleanup:
>      type = NULL;
>  
>      if (!ret && priv->open_recent_dialog) {
> +        if (error) {
in case of pointers we prefer an explicit comparison to NULL (just a
style - which is not followed...)

so it can be error != NULL && error->message != NULL to make everyone
happy. Although as you said the error->message is never checked...
otoh if the message is NULL then an empty dialog would appear.

> +            virt_viewer_app_simple_message_dialog(&self->parent, 
the first param can be app

> "%s", error->message);

the string will be displayed to the user, so it should be translated -
in case of the empty message, it should say something.

maybe "Unable to connect" error->message 

Thanks,
Pavel


P.S.: Looking at the code I can see dialogs having mentioned issues...

> +        }
>          g_clear_error(&error);
>          goto retry_dialog;
>      }




More information about the virt-tools-list mailing list