Re: [Spice-devel] [spice-gtk v2 2/7] channel-main: clipboard request: wakeup only when needed
Hi, On Tue, Dec 11, 2018 at 07:04:10PM +0100, Jakub Janku wrote: > Hi, > > On Mon, Dec 10, 2018 at 12:03 PM Victor Toso wrote: > > > > From: Victor Toso > > > > This patch makes agent_clipboard_request() 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_request() > > > > Signed-off-by: Victor Toso > > --- > > 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 4c6bc70..fb7175e 100644 > > --- a/src/channel-main.c > > +++ b/src/channel-main.c > > @@ -1419,23 +1419,24 @@ static void agent_clipboard_notify(SpiceMainChannel > > *self, guint selection, > > } > > > > /* any context: the message is not flushed immediately, > > - you can wakeup() the channel coroutine or send_msg_queue() */ > > -static void agent_clipboard_request(SpiceMainChannel *channel, guint > > selection, guint32 type) > > + you can wakeup() the channel coroutine or send_msg_queue() > > + Returns true if message was queued */ > > +static bool agent_clipboard_request(SpiceMainChannel *channel, guint > > selection, guint32 type) > > Might be better to use gboolean and TRUE/FALSE (instead of bool with > true/false) to match the rest of the file? Indeed, I was not considering the rest of the file while using bool. I'll change it. > Apart from that, it looks good to me. > > Cheers, > Jakub Thanks! > > > { > > SpiceMainChannelPrivate *c = channel->priv; > > VDAgentClipboardRequest *request; > > guint8 *msg; > > size_t msgsize; > > > > -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); > > > > msgsize = sizeof(VDAgentClipboardRequest); > > if (test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_SELECTION)) { > > msgsize += 4; > > } else if (selection != VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD) { > > SPICE_DEBUG("Ignoring clipboard request"); > > -return; > > +return false; > > } > > > > msg = g_alloca(msgsize); > > @@ -1451,6 +1452,7 @@ static void agent_clipboard_request(SpiceMainChannel > > *channel, guint selection, > > request->type = type; > > > > agent_msg_queue(channel, VD_AGENT_CLIPBOARD_REQUEST, msgsize, msg); > > +return true; > > } > > > > /* any context: the message is not flushed immediately, > > @@ -2936,8 +2938,9 @@ void > > spice_main_channel_clipboard_selection_request(SpiceMainChannel *channel, g > > g_return_if_fail(channel != NULL); > > g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel)); > > > > -agent_clipboard_request(channel, selection, type); > > -spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE); > > +if (agent_clipboard_request(channel, selection, type)) { > > +spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE); > > +} > > } > > > > /** > > -- > > 2.19.2 > > > > ___ > > Spice-devel mailing list > > Spice-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/spice-devel signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 2/7] channel-main: clipboard request: wakeup only when needed
Hi, On Mon, Dec 10, 2018 at 12:03 PM Victor Toso wrote: > > From: Victor Toso > > This patch makes agent_clipboard_request() 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_request() > > Signed-off-by: Victor Toso > --- > 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 4c6bc70..fb7175e 100644 > --- a/src/channel-main.c > +++ b/src/channel-main.c > @@ -1419,23 +1419,24 @@ static void agent_clipboard_notify(SpiceMainChannel > *self, guint selection, > } > > /* any context: the message is not flushed immediately, > - you can wakeup() the channel coroutine or send_msg_queue() */ > -static void agent_clipboard_request(SpiceMainChannel *channel, guint > selection, guint32 type) > + you can wakeup() the channel coroutine or send_msg_queue() > + Returns true if message was queued */ > +static bool agent_clipboard_request(SpiceMainChannel *channel, guint > selection, guint32 type) Might be better to use gboolean and TRUE/FALSE (instead of bool with true/false) to match the rest of the file? Apart from that, it looks good to me. Cheers, Jakub > { > SpiceMainChannelPrivate *c = channel->priv; > VDAgentClipboardRequest *request; > guint8 *msg; > size_t msgsize; > > -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); > > msgsize = sizeof(VDAgentClipboardRequest); > if (test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_SELECTION)) { > msgsize += 4; > } else if (selection != VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD) { > SPICE_DEBUG("Ignoring clipboard request"); > -return; > +return false; > } > > msg = g_alloca(msgsize); > @@ -1451,6 +1452,7 @@ static void agent_clipboard_request(SpiceMainChannel > *channel, guint selection, > request->type = type; > > agent_msg_queue(channel, VD_AGENT_CLIPBOARD_REQUEST, msgsize, msg); > +return true; > } > > /* any context: the message is not flushed immediately, > @@ -2936,8 +2938,9 @@ void > spice_main_channel_clipboard_selection_request(SpiceMainChannel *channel, g > g_return_if_fail(channel != NULL); > g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel)); > > -agent_clipboard_request(channel, selection, type); > -spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE); > +if (agent_clipboard_request(channel, selection, type)) { > +spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE); > +} > } > > /** > -- > 2.19.2 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 2/7] channel-main: clipboard request: wakeup only when needed
From: Victor Toso This patch makes agent_clipboard_request() 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_request() Signed-off-by: Victor Toso --- 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 4c6bc70..fb7175e 100644 --- a/src/channel-main.c +++ b/src/channel-main.c @@ -1419,23 +1419,24 @@ static void agent_clipboard_notify(SpiceMainChannel *self, guint selection, } /* any context: the message is not flushed immediately, - you can wakeup() the channel coroutine or send_msg_queue() */ -static void agent_clipboard_request(SpiceMainChannel *channel, guint selection, guint32 type) + you can wakeup() the channel coroutine or send_msg_queue() + Returns true if message was queued */ +static bool agent_clipboard_request(SpiceMainChannel *channel, guint selection, guint32 type) { SpiceMainChannelPrivate *c = channel->priv; VDAgentClipboardRequest *request; guint8 *msg; size_t msgsize; -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); msgsize = sizeof(VDAgentClipboardRequest); if (test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_SELECTION)) { msgsize += 4; } else if (selection != VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD) { SPICE_DEBUG("Ignoring clipboard request"); -return; +return false; } msg = g_alloca(msgsize); @@ -1451,6 +1452,7 @@ static void agent_clipboard_request(SpiceMainChannel *channel, guint selection, request->type = type; agent_msg_queue(channel, VD_AGENT_CLIPBOARD_REQUEST, msgsize, msg); +return true; } /* any context: the message is not flushed immediately, @@ -2936,8 +2938,9 @@ void spice_main_channel_clipboard_selection_request(SpiceMainChannel *channel, g g_return_if_fail(channel != NULL); g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel)); -agent_clipboard_request(channel, selection, type); -spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE); +if (agent_clipboard_request(channel, selection, type)) { +spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE); +} } /** -- 2.19.2 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel