Re: [spice-gtk v1] gtk-session: reply agent's clipboard request on failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> > > 
> > > 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/7b0de6217670e0f668aff2949f
> > 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@xxxxxxxxxx>
> > > ---
> > >  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.

>
> > > +
> > >      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).

>
> > > +        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@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]