Re: [spice-gtk v1 4/4] gtk-session: better variable name

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

 



> 
> From: Victor Toso <me@xxxxxxxxxxxxxx>
> 
> Not saying it is perfect name but 'i' as index does not state much
> after the for loop in g_memdup() and gtk_clipboard_set_with_owner().
> 
> Using number of elements as indexes is far from unusual so let's
> rename it and reduce by one the single letter vars.
> 
> Also note that the change in indentation on arguments of
> gtk_clipboard_set_with_owner() while renaming 'i' -> 'num_targets'.
> Follow up patch will rename some callbacks, so keeping one argument
> per line would reduce slightly the change set of patch set.
> 
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> ---
>  src/spice-gtk-session.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index bf3c1fb..ef3faea 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -781,22 +781,22 @@ static gboolean clipboard_grab(SpiceMainChannel *main,
> guint selection,
>      gboolean target_selected[SPICE_N_ELEMENTS(atom2agent)] = { FALSE, };
>      gboolean found;
>      GtkClipboard* cb;
> -    int m, n, i;
> +    int m, n;
> +    int num_targets = 0;
>  
>      cb = get_clipboard_from_selection(s, selection);
>      g_return_val_if_fail(cb != NULL, FALSE);
>  
> -    i = 0;
>      for (n = 0; n < ntypes; ++n) {
>          found = FALSE;
>          for (m = 0; m < SPICE_N_ELEMENTS(atom2agent); m++) {
>              if (atom2agent[m].vdagent == types[n] && !target_selected[m]) {
>                  found = TRUE;
> -                g_return_val_if_fail(i < SPICE_N_ELEMENTS(atom2agent),
> FALSE);
> -                targets[i].target = (gchar*)atom2agent[m].xatom;
> -                targets[i].info = m;
> +                g_return_val_if_fail(num_targets <
> SPICE_N_ELEMENTS(atom2agent), FALSE);
> +                targets[num_targets].target = (gchar*)atom2agent[m].xatom;
> +                targets[num_targets].info = m;
>                  target_selected[m] = TRUE;
> -                i += 1;
> +                num_targets += 1;

Really minor, why not "num_targets++;" ?

Very OT: sometimes I miss the "with" statement from Pascal!

>              }
>          }
>          if (!found) {
> @@ -806,8 +806,8 @@ static gboolean clipboard_grab(SpiceMainChannel *main,
> guint selection,
>      }
>  
>      g_free(s->clip_targets[selection]);
> -    s->nclip_targets[selection] = i;
> -    s->clip_targets[selection] = g_memdup(targets, sizeof(GtkTargetEntry) *
> i);
> +    s->nclip_targets[selection] = num_targets;
> +    s->clip_targets[selection] = g_memdup(targets, sizeof(GtkTargetEntry) *
> num_targets);
>      /* Receiving a grab implies we've released our own grab */
>      s->clip_grabbed[selection] = FALSE;
>  
> @@ -817,8 +817,12 @@ static gboolean clipboard_grab(SpiceMainChannel *main,
> guint selection,
>          return TRUE;
>      }
>  
> -    if (!gtk_clipboard_set_with_owner(cb, targets, i,
> -                                      clipboard_get, clipboard_clear,
> G_OBJECT(self))) {
> +    if (!gtk_clipboard_set_with_owner(cb,
> +                                      targets,
> +                                      num_targets,
> +                                      clipboard_get,
> +                                      clipboard_clear,
> +                                      G_OBJECT(self))) {
>          g_warning("clipboard grab failed");
>          return FALSE;
>      }

Otherwise,
Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx>

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




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