[Spice-devel] [PATCH spice-gtk 2/3] usb-device-manager: Don't show an error dialog on a cancelled device open

[Marc-André Lureau]

On Sat, Nov 19, 2011 at 4:29 PM, Hans de Goede <hdegoede at redhat.com> wrote:
> If for example the user plugs in a new device, then gets the policykit agent
> authentication dialog and then unplugs the device, spice-gtk will cancel
> the acl-helper request, which in turn will dismiss the policykit agent
> authentication dialog. Which is all a nice and smooth user experience,
> except that when this happens spice-gtk throws a dialog with an error

s/spice-gtk/spicy

> that the open was cancelled. Since a cancel usually is done deliberately
> (such as on the user unpluging the device) no error dialog should be thrown
> for it.

But then the client looses track of this action and error. If we
imagine a special client asking you to plug a smartcard reader, and
the user unplugged it immediately we may still want to show an
appropriate error dialog, such as keep the reader plugged (perhaps the
example isn't well chosen)

Anyway, it's easy to filter an error from the client side, and I think
it could be interesting for the client to decide what to do / to show
in such situation.

>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  gtk/usb-device-manager.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
> index b1d6c95..24f9ad1 100644
> --- a/gtk/usb-device-manager.c
> +++ b/gtk/usb-device-manager.c
> @@ -432,15 +432,17 @@ static void spice_usb_device_manager_auto_connect_cb(GObject      *gobject,
>     GError *err = NULL;
>
>     spice_usb_device_manager_connect_device_finish(self, res, &err);
> -    if (err) {
> +    if (err && !(err->domain == G_IO_ERROR &&
> +                 err->code   == G_IO_ERROR_CANCELLED)) {

So perhaps this filtering should be done in spicy.

>         gchar *desc = spice_usb_device_get_description(device);
>         g_prefix_error(&err, "Could not auto-redirect %s: ", desc);
>         g_free(desc);
>
>         g_warning("%s", err->message);
>         g_signal_emit(self, signals[AUTO_CONNECT_FAILED], 0, device, err);
> -        g_error_free(err);
>     }
> +    if (err)
> +        g_error_free(err);

In general, we should favor g_clear_error(), which is also NULL tolerant.


-- 
Marc-André Lureau