[spice-gtk,v2,4/7] channel-main: clipboard release: wakeup only when needed

Submitted by Victor Toso on Dec. 10, 2018, 11:02 a.m.

Details

Message ID 20181210110256.20115-5-victortoso@redhat.com
State New
Headers show
Series "small code improvements on clipboard" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Victor Toso Dec. 10, 2018, 11:02 a.m.
From: Victor Toso <me@victortoso.com>

This patch makes agent_clipboard_release() to return true only when
the message request to the agent is successfully queued to be sent.

By checking the return value, we can avoid wakeup the channel
unnecessarily at spice_main_channel_clipboard_selection_release()

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 src/channel-main.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/channel-main.c b/src/channel-main.c
index 21afd87..5bcd149 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -1458,25 +1458,27 @@  static bool agent_clipboard_request(SpiceMainChannel *channel, guint selection,
 }
 
 /* any context: the message is not flushed immediately,
-   you can wakeup() the channel coroutine or send_msg_queue() */
-static void agent_clipboard_release(SpiceMainChannel *channel, guint selection)
+   you can wakeup() the channel coroutine or send_msg_queue()
+   Returns true if message was queued */
+static bool agent_clipboard_release(SpiceMainChannel *channel, guint selection)
 {
     SpiceMainChannelPrivate *c = channel->priv;
     guint8 msg[4] = { 0, };
     guint8 msgsize = 0;
 
-    g_return_if_fail(c->agent_connected);
-    g_return_if_fail(test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND));
+    g_return_val_if_fail(c->agent_connected, false);
+    g_return_val_if_fail(test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND), false);
 
     if (test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_SELECTION)) {
         msg[0] = selection;
         msgsize += 4;
     } else if (selection != VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD) {
         SPICE_DEBUG("Ignoring clipboard release");
-        return;
+        return false;
     }
 
     agent_msg_queue(channel, VD_AGENT_CLIPBOARD_RELEASE, msgsize, msg);
+    return true;
 }
 
 static gboolean any_display_has_dimensions(SpiceMainChannel *channel)
@@ -2828,8 +2830,9 @@  void spice_main_channel_clipboard_selection_release(SpiceMainChannel *channel, g
     if (!c->agent_connected)
         return;
 
-    agent_clipboard_release(channel, selection);
-    spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
+    if (agent_clipboard_release(channel, selection)) {
+        spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
+    }
 }
 
 /**

Comments

On 12/10/18 1:02 PM, Victor Toso wrote:
> From: Victor Toso <me@victortoso.com>
> 
> This patch makes agent_clipboard_release() to return true only when
> the message request to the agent is successfully queued to be sent.
> 
> By checking the return value, we can avoid wakeup the channel
> unnecessarily at spice_main_channel_clipboard_selection_release()
> 
> Signed-off-by: Victor Toso <victortoso@redhat.com>

Looks good to me.

Uri.

> ---
>   src/channel-main.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 21afd87..5bcd149 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -1458,25 +1458,27 @@ static bool agent_clipboard_request(SpiceMainChannel *channel, guint selection,
>   }
>   
>   /* any context: the message is not flushed immediately,
> -   you can wakeup() the channel coroutine or send_msg_queue() */
> -static void agent_clipboard_release(SpiceMainChannel *channel, guint selection)
> +   you can wakeup() the channel coroutine or send_msg_queue()
> +   Returns true if message was queued */
> +static bool agent_clipboard_release(SpiceMainChannel *channel, guint selection)
>   {
>       SpiceMainChannelPrivate *c = channel->priv;
>       guint8 msg[4] = { 0, };
>       guint8 msgsize = 0;
>   
> -    g_return_if_fail(c->agent_connected);
> -    g_return_if_fail(test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND));
> +    g_return_val_if_fail(c->agent_connected, false);
> +    g_return_val_if_fail(test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND), false);
>   
>       if (test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_SELECTION)) {
>           msg[0] = selection;
>           msgsize += 4;
>       } else if (selection != VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD) {
>           SPICE_DEBUG("Ignoring clipboard release");
> -        return;
> +        return false;
>       }
>   
>       agent_msg_queue(channel, VD_AGENT_CLIPBOARD_RELEASE, msgsize, msg);
> +    return true;
>   }
>   
>   static gboolean any_display_has_dimensions(SpiceMainChannel *channel)
> @@ -2828,8 +2830,9 @@ void spice_main_channel_clipboard_selection_release(SpiceMainChannel *channel, g
>       if (!c->agent_connected)
>           return;
>   
> -    agent_clipboard_release(channel, selection);
> -    spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
> +    if (agent_clipboard_release(channel, selection)) {
> +        spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
> +    }
>   }
>   
>   /**
>