On Wed, 2017-05-24 at 18:54 +0200, Victor Toso wrote:
> Hi,
>
> On Wed, May 24, 2017 at 11:36:55AM -0500, Jonathon Jongsma wrote:
> > I see Frediano acked this, but there are a couple issues with the
> > commit log if you haven't pushed yet.
> >
> > On Mon, 2017-05-22 at 20:08 +0200, Victor Toso wrote:
> > > From: Victor Toso <me@xxxxxxxxxxxxxx>
> > >
> > > By using 'num_types' variables, we have a clear variable with a
> > > clear
> > > propose: It will track the number of VD_AGENT_CLIPBOARD types we
> > > are
> >
> > I think you mean purpose instead of propose?
>
> Yes!
>
> >
> > > storing in types[] array.
> > >
> > > This new variable helps:
> > > - removing one for that was counting the number stored types;
> >
> > "for that was" is a very confusing phrase. It took me about 3 re-
> > readings (and a look at the patch) to decipher that the word "for"
> > here
> > referred to the C loop keyword. Perhaps consider "loop that was" or
> > "for loop that was"
>
> Sure
>
> >
> > > - reducing one for to the size of 'num_types'
> >
> > I guess here you're trying to say that you're doing fewer
> > iterations in
> > a 'for' loop, right?
>
> Yes!
>
> Does it look better?
>
> "
> gtk-session: use clear variable for array's size
>
> By using 'num_types' variables, we have a clear variable with a clear
> purpose: It will track the number of VD_AGENT_CLIPBOARD types we are
> storing in types[] array.
>
> This new variable helps:
> - removing one 'for' loop which was counting the number stored types;
> - doing fewer iterations in one 'for' loop
>
> A few extra comments were included to clarify what the logic should
> be
> doing and a extra debug was included to point out situations where
> the
> desktop has sent us valid clipboard data but none will be sent the
> guest.
>
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> "
>
> Many thanks!
Looks good to me!
Jonathon
>
> >
> >
> > >
> > > A few extra comments were included to clarify what the logic
> > > should
> > > be
> > > doing and a extra debug was included to point out situations
> > > where
> > > the
> > > desktop has sent us valid clipboard data but none will be sent
> > > the
> > > guest.
> > >
> > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > > Signed-off-by: Victor Toso <me@xxxxxxxxxxxxxx>
> > > ---
> > > src/spice-gtk-session.c | 34 +++++++++++++++++++---------------
> > > 1 file changed, 19 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > index cfdfc4c..83eaa3e 100644
> > > --- a/src/spice-gtk-session.c
> > > +++ b/src/spice-gtk-session.c
> > > @@ -616,6 +616,7 @@ static void
> > > clipboard_get_targets(GtkClipboard
> > > *clipboard,
> > >
> > > SpiceGtkSessionPrivate *s = self->priv;
> > > guint32 types[SPICE_N_ELEMENTS(atom2agent)] = { 0 };
> > > + gint num_types;
> > > char *name;
> > > int a, m, t;
> > > int selection;
> > > @@ -635,38 +636,41 @@ static void
> > > clipboard_get_targets(GtkClipboard
> > > *clipboard,
> > > }
> > > }
> > >
> > > + /* Set all Atoms that matches our current protocol
> > > implementation */
> > > + num_types = 0;
> > > for (a = 0; a < n_atoms; a++) {
> > > name = gdk_atom_name(atoms[a]);
> > > for (m = 0; m < SPICE_N_ELEMENTS(atom2agent); m++) {
> > > if (strcasecmp(name, atom2agent[m].xatom) != 0) {
> > > continue;
> > > }
> > > - /* found match */
> > > - for (t = 0; t < SPICE_N_ELEMENTS(atom2agent); t++) {
> > > +
> > > + /* check if type is already in list */
> > > + for (t = 0; t < num_types; t++) {
> > > if (types[t] == atom2agent[m].vdagent) {
> > > - /* type already in list */
> > > - break;
> > > - }
> > > - if (types[t] == 0) {
> > > - /* add type to empty slot */
> > > - types[t] = atom2agent[m].vdagent;
> > > break;
> > > }
> > > }
> > > - break;
> > > +
> > > + if (types[t] == 0) {
> > > + /* add type to empty slot */
> > > + types[t] = atom2agent[m].vdagent;
> > > + num_types++;
> > > + }
> > > }
> > > g_free(name);
> > > }
> > > - for (t = 0; t < SPICE_N_ELEMENTS(atom2agent); t++) {
> > > - if (types[t] == 0) {
> > > - break;
> > > - }
> > > +
> > > + if (num_types == 0) {
> > > + SPICE_DEBUG("No GdkAtoms will be sent from %d",
> > > n_atoms);
> > > + return;
> > > }
> > > - if (!s->clip_grabbed[selection] && t > 0) {
> > > +
> > > + if (!s->clip_grabbed[selection]) {
> > > s->clip_grabbed[selection] = TRUE;
> > >
> > > if (spice_main_agent_test_capability(s->main,
> > > VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
> > > - spice_main_clipboard_selection_grab(s->main,
> > > selection,
> > > types, t);
> > > + spice_main_clipboard_selection_grab(s->main,
> > > selection,
> > > types, num_types);
> > > /* Sending a grab causes the agent to do an implicit
> > > release
> > > */
> > > s->nclip_targets[selection] = 0;
> > > }
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel