[Spice-devel] [PATCH vdagent v4 3/4] refetch _NET_WM_NAME in vdagent_x11_has_icons_on_desktop()

Victor Toso victortoso at redhat.com
Wed Dec 6 15:18:25 UTC 2017


Hi,

On Tue, Dec 05, 2017 at 07:57:16PM +0100, Jakub Janků wrote:
> On Tue, 2017-12-05 at 19:45 +0100, Jakub Janků wrote:
> > Don't cache _NET_WM_NAME, instead refetch it with each
> > vdagent_x11_has_icons_on_desktop() call so that correct result is
> > returned
> > even if the window manager changes during the lifetime of vdagent_x11
> > struct.
> > Remove char *net_wm_name from vdagent_x11.
> > 
> > Remove x11 argument from vdagent_x11_has_icons_on_desktop().
> > 
> > Since the name is refetched, don't log it in vdagent_x11_create().
> 
> Oh, I forgot to remove this. Since v4, the logging is on Victor's
> recommendation back, adding the name of function "vdagent_x11_create".

Yep, plus adding 'x11: ' I think, would fit okay in the shortlog and...

> 
> Sorry,
> Jakub
> 
> > ---
> >  src/vdagent/vdagent.c  |  8 ++++----
> >  src/vdagent/x11-priv.h |  1 -
> >  src/vdagent/x11.c      | 24 ++++++++++++------------
> >  src/vdagent/x11.h      |  2 +-
> >  4 files changed, 17 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> > index 4710a44..d86ee25 100644
> > --- a/src/vdagent/vdagent.c
> > +++ b/src/vdagent/vdagent.c
> > @@ -109,7 +109,7 @@ static const gchar
> > *xfer_get_download_directory(VDAgent *agent)
> >          return fx_dir;
> >      }
> >  
> > -    return
> > g_get_user_special_dir(vdagent_x11_has_icons_on_desktop(agent->x11) ?
> > +    return g_get_user_special_dir(vdagent_x11_has_icons_on_desktop()
> > ?
> >                                    G_USER_DIRECTORY_DESKTOP :
> >                                    G_USER_DIRECTORY_DOWNLOAD);
> >  }
> > @@ -123,6 +123,7 @@ static const gchar
> > *xfer_get_download_directory(VDAgent *agent)
> >  static gboolean vdagent_init_file_xfer(VDAgent *agent)
> >  {
> >      const gchar *xfer_dir;
> > +    gboolean open_dir;
> >  
> >      if (agent->xfers != NULL) {
> >          syslog(LOG_DEBUG, "File-xfer already initialized");
> > @@ -137,11 +138,10 @@ static gboolean vdagent_init_file_xfer(VDAgent
> > *agent)
> >          return FALSE;
> >      }
> >  
> > -    if (fx_open_dir == -1)
> > -        fx_open_dir = !vdagent_x11_has_icons_on_desktop(agent->x11);
> > +    open_dir = fx_open_dir == -1 ?
> > !vdagent_x11_has_icons_on_desktop() : fx_open_dir;
> >  
> >      agent->xfers = vdagent_file_xfers_create(agent->conn, xfer_dir,
> > -                                             fx_open_dir, debug);
> > +                                             open_dir, debug);
> >      return (agent->xfers != NULL);
> >  }
> >  
> > diff --git a/src/vdagent/x11-priv.h b/src/vdagent/x11-priv.h
> > index 677a44d..3776098 100644
> > --- a/src/vdagent/x11-priv.h
> > +++ b/src/vdagent/x11-priv.h
> > @@ -87,7 +87,6 @@ struct vdagent_x11 {
> >      Window root_window[MAX_SCREENS];
> >      Window selection_window;
> >      struct udscs_connection *vdagentd;
> > -    char *net_wm_name;
> >      int debug;
> >      int fd;
> >      int screen_count;
> > diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> > index ec068f4..fcbd01d 100644
> > --- a/src/vdagent/x11.c
> > +++ b/src/vdagent/x11.c
> > @@ -228,18 +228,18 @@ struct vdagent_x11 *vdagent_x11_create(struct
> > udscs_connection *vdagentd,
> >      }
> >      vdagent_x11_send_daemon_guest_xorg_res(x11, 1);
> >  
> > -    /* Get net_wm_name, since we are started at the same time as the
> > wm,
> > -       sometimes we need to wait a bit for it to show up. */
> > -    i = 10;
> > -    x11->net_wm_name = (gchar *)vdagent_x11_get_wm_name();
> > -    while (!strcmp(x11->net_wm_name, "unknown") && --i > 0) {
> > +    /* Since we are started at the same time as the wm,
> > +       sometimes we need to wait a bit for the _NET_WM_NAME to show
> > up. */
> > +    const gchar *net_wm_name;

... moving this to the top of the block, but I can do both things before
pushing, no need to send a v5 for this.

> > +    for (i = 0; i < 9; i++) {
> > +        net_wm_name = vdagent_x11_get_wm_name();
> > +        if (strcmp(net_wm_name, "unknown"))
> > +            break;
> >          usleep(100000);
> > -        x11->net_wm_name = (gchar *)vdagent_x11_get_wm_name();
> >      }
> > -    x11->net_wm_name = g_strdup(x11->net_wm_name);
> >      if (x11->debug)
> > -        syslog(LOG_DEBUG, "net_wm_name: \"%s\", has icons: %d",
> > -               x11->net_wm_name,
> > vdagent_x11_has_icons_on_desktop(x11));
> > +        syslog(LOG_DEBUG, "%s: net_wm_name=\"%s\", has icons=%d",
> > +               __func__, net_wm_name,
> > vdagent_x11_has_icons_on_desktop(x11));
> >  
> >      /* Flush output buffers and consume any pending events */
> >      vdagent_x11_do_read(x11);
> > @@ -262,7 +262,6 @@ void vdagent_x11_destroy(struct vdagent_x11 *x11,
> > int vdagentd_disconnected)
> >      }
> >  
> >      XCloseDisplay(x11->display);
> > -    g_free(x11->net_wm_name);
> >      free(x11->randr.failed_conf);
> >      free(x11);
> >  }
> > @@ -1291,7 +1290,7 @@ void vdagent_x11_client_disconnected(struct
> > vdagent_x11 *x11)
> >     whitelist approach, so any unknown desktop will end up with
> > saving
> >     file-xfers to the xdg download dir, and opening the xdg download
> > dir with
> >     xdg-open when the file-xfer completes. */
> > -int vdagent_x11_has_icons_on_desktop(struct vdagent_x11 *x11)
> > +int vdagent_x11_has_icons_on_desktop()
> >  {
> >      const char * const wms_with_icons_on_desktop[] = {
> >          "Metacity", /* GNOME-2 or GNOME-3 fallback */
> > @@ -1299,10 +1298,11 @@ int vdagent_x11_has_icons_on_desktop(struct
> > vdagent_x11 *x11)
> >          "Marco",    /* Mate */
> >          NULL
> >      };
> > +    const gchar *net_wm_name = vdagent_x11_get_wm_name();
> >      int i;
> >  
> >      for (i = 0; wms_with_icons_on_desktop[i]; i++)
> > -        if (!strcmp(x11->net_wm_name, wms_with_icons_on_desktop[i]))
> > +        if (!strcmp(net_wm_name, wms_with_icons_on_desktop[i]))
> >              return 1;
> >  
> >      return 0;
> > diff --git a/src/vdagent/x11.h b/src/vdagent/x11.h
> > index 4fd0380..a8ceb08 100644
> > --- a/src/vdagent/x11.h
> > +++ b/src/vdagent/x11.h
> > @@ -48,6 +48,6 @@ void vdagent_x11_clipboard_release(struct
> > vdagent_x11 *x11, uint8_t selection);
> >  
> >  void vdagent_x11_client_disconnected(struct vdagent_x11 *x11);
> >  
> > -int vdagent_x11_has_icons_on_desktop(struct vdagent_x11 *x11);
> > +int vdagent_x11_has_icons_on_desktop();
> >  
> >  #endif
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20171206/702b1b6d/attachment.sig>


More information about the Spice-devel mailing list