[Spice-devel] [PATCH] vdagent: when SetClipboardData fails, set clipboard control event to stop wait

[Uri Lublin]

On 11/27/2012 11:16 AM, Arnon Gilboa wrote:
> Currently, a SetClipboardData failure is followed by useless wait for timeout
> (VD_CLIPBOARD_TIMEOUT_MS)
> ---
>   vdagent/vdagent.cpp |   30 ++++++++++++++++--------------
>   1 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
> index 3f1053d..2bb466d 100644
> --- a/vdagent/vdagent.cpp
> +++ b/vdagent/vdagent.cpp
> @@ -643,12 +643,10 @@ bool VDAgent::handle_clipboard(VDAgentClipboard* clipboard, uint32_t size)
>
>       if (_clipboard_owner != owner_client) {
>           vd_printf("Received clipboard data from client while clipboard is not owned by client");
> -        set_control_event(CONTROL_CLIPBOARD);
> -        return false;
> +        goto fin;
>       }
>       if (clipboard->type == VD_AGENT_CLIPBOARD_NONE) {
> -        set_control_event(CONTROL_CLIPBOARD);
> -        return false;
> +        goto fin;
>       }
>       switch (clipboard->type) {
>       case VD_AGENT_CLIPBOARD_UTF8_TEXT:
> @@ -664,20 +662,24 @@ bool VDAgent::handle_clipboard(VDAgentClipboard* clipboard, uint32_t size)
>       }
>       default:
>           vd_printf("Unsupported clipboard type %u", clipboard->type);
> -        return true;
> +        goto fin;
>       }
>       format = get_clipboard_format(clipboard->type);
> -    if (SetClipboardData(format, clip_data)) {
> -        set_control_event(CONTROL_CLIPBOARD);
> -        return true;
> -    }
> -    // We retry clipboard open-empty-set-close only when there is a timeout in on_clipboard_request()
> -    if (!OpenClipboard(_hwnd)) {
> -        return false;
> +    if (format == 0) {
> +        vd_printf("Unknown clipboard format, type %u", clipboard->type);
> +        goto fin;
>       }
> -    EmptyClipboard();
>       ret = !!SetClipboardData(format, clip_data);
> -    CloseClipboard();
> +    if (!ret) {
> +        DWORD err = GetLastError();
> +        if (err == ERROR_NOT_ENOUGH_MEMORY) {
> +            vd_printf("Not enough memory to set clipboard data, size %u bytes", size);
> +        } else {
> +            vd_printf("SetClipboardData failed: %u", err);
> +        }
> +    }
> +fin:
> +    set_control_event(CONTROL_CLIPBOARD);
>       return ret;
>   }
>

Ack.

Two comments:
- Only the 'default' case changed (set event + return false), other 
cases are
   logically the same (but it's a nice cleanup for them too).
- retry clipboard open-empty-set-close was removed.
   would be nice to add it to the log message (or as a separate patch).