[Spice-devel] [PATCH spice-gtk v2 1/4] gtk-session: use clear variable for array's size

[Frediano Ziglio]

> 
> From: Victor Toso <me at victortoso.com>
> 
> There is no need to use an index variable to keep track of the number
> of VD_AGENT_CLIPBOARD types we are storing.
> 

I got to read the code.
Basically the "t" variable was used to compute the number of
items stored inside "types", so yes, "num_types" sounds a much better name.

> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
>  src/spice-gtk-session.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index cfdfc4c..2ae512f 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -619,6 +619,7 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
>      char *name;
>      int a, m, t;
>      int selection;
> +    gint num_types;
>  
>      if (s->main == NULL)
>          return;

I would use "gint num_types = 0;" just as is the number of elements of "types"
and maybe I would put it just under the "types" declaration.
At this point there's also no need to initialize "types"... but this
require a bit of changes... see below (not really mandatory).

> @@ -635,6 +636,8 @@ 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++) {

The similar for loop (using "t" index) should be limited to num_types.

> @@ -650,6 +653,7 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
>                  if (types[t] == 0) {

And this test should be changed (and possibly moved outside the for).

>                      /* add type to empty slot */
>                      types[t] = atom2agent[m].vdagent;
> +                    num_types++;
>                      break;
>                  }
>              }
> @@ -657,16 +661,17 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
>          }
>          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;
>      }

This debug was not present... but is just a debug.

> -    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;
>      }

Otherwise looks fine

Frediano