[Spice-devel] [spice-gtk v1] gtk-session: reply agent's clipboard request on failure
Pavel Grunt
pgrunt at redhat.com
Fri Feb 24 10:04:38 UTC 2017
On Fri, 2017-02-24 at 10:49 +0100, Victor Toso wrote:
> Hi,
>
> On Fri, Feb 24, 2017 at 10:25:43AM +0100, Pavel Grunt wrote:
> > Hi,
> >
> > On Fri, 2017-02-24 at 10:07 +0100, Victor Toso wrote:
> > > Hi,
> > >
> > > On Wed, Feb 22, 2017 at 01:29:20PM +0100, Victor Toso wrote:
> >
> > Maybe "gtk-session: Always reply to agent's clipboard request"
> > fits
> > better
>
> Ok, fixed locally
>
> >
> > > > From: Victor Toso <me at victortoso.com>
> > > >
> > > > We need to reply back to the agent all clipboard requests even
> > > > in
> > > > case
> > > > of failure otherwise it will have a pending request. The
> > > > following
> > > > error message can be seen in afterwards, when client sends
> > > > down
> > > > some
> > > > clipboard data:
> > >
> > > It is worth to mention in the commit log that this is a
> > > regression
> > > from
> > > 7b0de6217670e0f668aff2949f, as we were notifying the agent even
> > > on
> > > failure, see:
> >
> > thanks
> > >
> > > https://gitlab.com/spice/spice-gtk/commit/7b0de6217670e0f668aff2
> > > 949f
> > > ba174ed3cc0b50#7d95131068c82b7224989c1c61cc3c85a9743df3_956_990
> > >
> > > I've included in the commit log:
> > >
> > > "This fixes a regression from 7b0de6217670e0f668aff2949f"
> > >
> > > Cheers,
> > >
> > > >
> > > > > clipboard: selection requests pending on clipboard
> > > > ownership
> > > > > change, clearing
> > > >
> > > > An easy way to reproduce this is:
> > > > 1-) In client, copy image from lo-draw (selection or ctrl+c)
> > > > 2-) In guest, paste it to GEdit (mouse3 our ctrl+v)
> > > > 3-) Move to the client
> > > > 4-) Move back to the guest
> > > > 5-) see error on vdagent logs
> > > >
> >
> > This seems to be specific to the linux agent
>
> Yes, we could improve the linux agent too if possible, in the future
> but
> it makes sense to me that we reply an agent's request for clipboard
> data even if it is empty.
>
> >
> > > > The reason for failure is that client's clipboard contains
> > > > different
> > > > data type (image) then what was requested from guest's editor
> > > > (text)
> > > >
> > > > While at it, include extra debug message as it might be hard
> > > > to
> > > > identify why clipboard did not work.
> > > >
> > > > Resolves: rhbz#1409854
> > > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > > ---
> > > > src/spice-gtk-session.c | 19 ++++++++++++-------
> > > > 1 file changed, 12 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > > index 0426d8f..315bc26 100644
> > > > --- a/src/spice-gtk-session.c
> > > > +++ b/src/spice-gtk-session.c
> > > > @@ -940,28 +940,33 @@ static void
> > > > clipboard_received_text_cb(GtkClipboard *clipboard,
> > > > if (self == NULL)
> > > > return;
> > > >
> > > > + selection = get_selection_from_clipboard(self->priv,
> > > > clipboard);
> > > > + g_return_if_fail(selection != -1);
> >
> > so is it not needed to notify here as well ?
>
> We can't without knowing the selection.
hmhmhm, you can keep it, it is not related to the patch
the g_return is not needed, selection is used by a function in channel
main, and that function should verify it. There can be more
improvements (clipboard_received_text_cb is called from
clipboard_request which has info about the selection)
>
> >
> > > > +
> > > > if (text == NULL) {
> > > > SPICE_DEBUG("Failed to retrieve clipboard text");
> > > > - return;
> > > > + goto notify_agent;
> > > > }
> > > >
> > > > g_return_if_fail(SPICE_IS_GTK_SESSION(self));
> > > >
> > > > - selection = get_selection_from_clipboard(self->priv,
> > > > clipboard);
> > > > - g_return_if_fail(selection != -1);
> > > > -
> > > > len = strlen(text);
> > > > if (!check_clipboard_size_limits(self, len)) {
> > > > - return;
> > > > + SPICE_DEBUG("Failed sized limits of clipboard text
> > > > (%d
> > > > bytes)", len);
> > > > + len = 0;
> > > > + goto notify_agent;
> > > > }
> > > >
> > > > /* gtk+ internal utf8 newline is always LF, even on
> > > > windows
> > > > */
> > > > conv = fixup_clipboard_text(self, text, &len);
> > > > if (!check_clipboard_size_limits(self, len)) {
> > > > - g_free(conv);
> > > > - return;
> > > > + SPICE_DEBUG("Failed sized limits of clipboard text
> > > > (%d
> > > > bytes)", len);
> > > > + g_clear_pointer(&conv, g_free);
> >
> > thanks to goto there is no need to clear it
>
> We do. Conv might not have failed, this check checks the
> clipboard_size_limits so, if limit exceed the protocol limit, we
> need to
> free conv here to send the right value to agent (nothing).
it sends len = 0
>
> >
> > > > + len = 0;
> > > > + goto notify_agent;
> > > > }
> > > >
> > > > +notify_agent:
> > > > spice_main_clipboard_selection_notify(self->priv->main,
> > > > selection,
> > > > VD_AGENT_CLIPBOARD_
> > > > UTF8
> > > > _TEXT,
> > > > (guchar *)(conv ?:
> > > > text), len);
> > > > --
> > > > 2.9.3
> > > >
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > >
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list