[Spice-devel,spice-gtk,v1,5/7] gtk-session: move atom's debug

Submitted by Victor Toso on Feb. 22, 2017, 1:11 p.m.

Details

Message ID 20170222131112.14079-6-victortoso@redhat.com
State New
Headers show
Series "clipboard_get_targets() on spice-gtk-session.c" ( rev: 2 1 ) in Spice

Not browsing as part of any series.

Commit Message

Victor Toso Feb. 22, 2017, 1:11 p.m.
From: Victor Toso <me@victortoso.com>

We already iterate in all n_atoms and get its name with
gdk_atom_name(), let's move the debug there too;

As spice_util_get_debug() uses g_once(), this should not impact
performance or might improve it with one memory allocation less.

While at it, move the generic SPICE_DEBUG of the function to the top.
It should help us identify when the function was triggered but not
used due the early returns.

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 src/spice-gtk-session.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 651e438..8682229 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -604,6 +604,7 @@  static void clipboard_get_targets(GtkClipboard *clipboard,
 {
     SpiceGtkSession *self = free_weak_ref(user_data);
 
+    SPICE_DEBUG("%s:", __FUNCTION__);
 
     if (self == NULL || atoms == NULL)
         return;
@@ -628,15 +629,6 @@  static void clipboard_get_targets(GtkClipboard *clipboard,
         return;
     }
 
-    SPICE_DEBUG("%s:", __FUNCTION__);
-    if (spice_util_get_debug()) {
-        for (a = 0; a < n_atoms; a++) {
-            name = gdk_atom_name(atoms[a]);
-            SPICE_DEBUG(" \"%s\"", name);
-            g_free(name);
-        }
-    }
-
     /* Set all Atoms that matches our current protocol implementation */
     num_types = 0;
     for (a = 0; a < n_atoms; a++) {
@@ -653,6 +645,7 @@  static void clipboard_get_targets(GtkClipboard *clipboard,
                 }
                 if (types[t] == 0) {
                     /* add type to empty slot */
+                    SPICE_DEBUG("'%s'", name);
                     types[t] = atom2agent[m].vdagent;
                     num_types++;
                     break;

Comments

On Wed, 2017-02-22 at 14:11 +0100, Victor Toso wrote:
> From: Victor Toso <me@victortoso.com>
> 
> We already iterate in all n_atoms and get its name with
> gdk_atom_name(), let's move the debug there too;
> 
> As spice_util_get_debug() uses g_once(), this should not impact
> performance or might improve it with one memory allocation less.
> 
> While at it, move the generic SPICE_DEBUG of the function to the
> top.
> It should help us identify when the function was triggered but not
> used due the early returns.
> 
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>  src/spice-gtk-session.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 651e438..8682229 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -604,6 +604,7 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
>  {
>      SpiceGtkSession *self = free_weak_ref(user_data);
>  
> +    SPICE_DEBUG("%s:", __FUNCTION__);
>  
>      if (self == NULL || atoms == NULL)
>          return;
> @@ -628,15 +629,6 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
>          return;
>      }
>  
> -    SPICE_DEBUG("%s:", __FUNCTION__);
> -    if (spice_util_get_debug()) {
> -        for (a = 0; a < n_atoms; a++) {
> -            name = gdk_atom_name(atoms[a]);
> -            SPICE_DEBUG(" \"%s\"", name);
> -            g_free(name);
> -        }
> -    }
> -
>      /* Set all Atoms that matches our current protocol
> implementation */
>      num_types = 0;
>      for (a = 0; a < n_atoms; a++) {
> @@ -653,6 +645,7 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
>                  }
>                  if (types[t] == 0) {
>                      /* add type to empty slot */
> +                    SPICE_DEBUG("'%s'", name);
It should go just after the gdk_atom_name() call otherwise you may
have a different result. Also please keep "" instead of ''

Pavel

>                      types[t] = atom2agent[m].vdagent;
>                      num_types++;
>                      break;
On Thu, 2017-02-23 at 14:15 +0100, Pavel Grunt wrote:
> On Wed, 2017-02-22 at 14:11 +0100, Victor Toso wrote:
> > From: Victor Toso <me@victortoso.com>
> > 
> > We already iterate in all n_atoms and get its name with
> > gdk_atom_name(), let's move the debug there too;
> > 
> > As spice_util_get_debug() uses g_once(), this should not impact
> > performance or might improve it with one memory allocation less.
> > 
> > While at it, move the generic SPICE_DEBUG of the function to the
> > top.
> > It should help us identify when the function was triggered but not
> > used due the early returns.
> > 
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > ---
> >  src/spice-gtk-session.c | 11 ++---------
> >  1 file changed, 2 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 651e438..8682229 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -604,6 +604,7 @@ static void clipboard_get_targets(GtkClipboard
> > *clipboard,
> >  {
> >      SpiceGtkSession *self = free_weak_ref(user_data);
> >  
> > +    SPICE_DEBUG("%s:", __FUNCTION__);
> >  
> >      if (self == NULL || atoms == NULL)
> >          return;
> > @@ -628,15 +629,6 @@ static void
> > clipboard_get_targets(GtkClipboard
> > *clipboard,
> >          return;
> >      }
> >  
> > -    SPICE_DEBUG("%s:", __FUNCTION__);
> > -    if (spice_util_get_debug()) {
> > -        for (a = 0; a < n_atoms; a++) {
> > -            name = gdk_atom_name(atoms[a]);
> > -            SPICE_DEBUG(" \"%s\"", name);
> > -            g_free(name);
> > -        }
> > -    }
> > -
> >      /* Set all Atoms that matches our current protocol
> > implementation */
> >      num_types = 0;
> >      for (a = 0; a < n_atoms; a++) {
> > @@ -653,6 +645,7 @@ static void clipboard_get_targets(GtkClipboard
> > *clipboard,
> >                  }
> >                  if (types[t] == 0) {
> >                      /* add type to empty slot */
> > +                    SPICE_DEBUG("'%s'", name);
> 
> It should go just after the gdk_atom_name() call otherwise you may
> have a different result. Also please keep "" instead of ''

And the indentation - imo it is easier to read

SPICE_DEBUG(" \"%s\"", name);

> 
> Pavel
> 
> >                      types[t] = atom2agent[m].vdagent;
> >                      num_types++;
> >                      break;
Hi,

On Thu, Feb 23, 2017 at 02:19:31PM +0100, Pavel Grunt wrote:
> On Thu, 2017-02-23 at 14:15 +0100, Pavel Grunt wrote:
> > On Wed, 2017-02-22 at 14:11 +0100, Victor Toso wrote:
> > > From: Victor Toso <me@victortoso.com>
> > > 
> > > We already iterate in all n_atoms and get its name with
> > > gdk_atom_name(), let's move the debug there too;
> > > 
> > > As spice_util_get_debug() uses g_once(), this should not impact
> > > performance or might improve it with one memory allocation less.
> > > 
> > > While at it, move the generic SPICE_DEBUG of the function to the
> > > top.
> > > It should help us identify when the function was triggered but not
> > > used due the early returns.
> > > 
> > > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > > ---
> > >  src/spice-gtk-session.c | 11 ++---------
> > >  1 file changed, 2 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > index 651e438..8682229 100644
> > > --- a/src/spice-gtk-session.c
> > > +++ b/src/spice-gtk-session.c
> > > @@ -604,6 +604,7 @@ static void clipboard_get_targets(GtkClipboard
> > > *clipboard,
> > >  {
> > >      SpiceGtkSession *self = free_weak_ref(user_data);
> > >  
> > > +    SPICE_DEBUG("%s:", __FUNCTION__);
> > >  
> > >      if (self == NULL || atoms == NULL)
> > >          return;
> > > @@ -628,15 +629,6 @@ static void
> > > clipboard_get_targets(GtkClipboard
> > > *clipboard,
> > >          return;
> > >      }
> > >  
> > > -    SPICE_DEBUG("%s:", __FUNCTION__);
> > > -    if (spice_util_get_debug()) {
> > > -        for (a = 0; a < n_atoms; a++) {
> > > -            name = gdk_atom_name(atoms[a]);
> > > -            SPICE_DEBUG(" \"%s\"", name);
> > > -            g_free(name);
> > > -        }
> > > -    }
> > > -
> > >      /* Set all Atoms that matches our current protocol
> > > implementation */
> > >      num_types = 0;
> > >      for (a = 0; a < n_atoms; a++) {
> > > @@ -653,6 +645,7 @@ static void clipboard_get_targets(GtkClipboard
> > > *clipboard,
> > >                  }
> > >                  if (types[t] == 0) {
> > >                      /* add type to empty slot */
> > > +                    SPICE_DEBUG("'%s'", name);
> >
> > It should go just after the gdk_atom_name() call otherwise you may
> > have a different result. Also please keep "" instead of ''

Ah, but if we do it here instead, we know the atoms that we are
storing/sending.

>
> And the indentation - imo it is easier to read
> 
> SPICE_DEBUG(" \"%s\"", name);

I prefer '' but I don't mind changing.

> 
> > 
> > Pavel
> > 
> > >                      types[t] = atom2agent[m].vdagent;
> > >                      num_types++;
> > >                      break;