[Spice-devel] [PATCH vdagent 2/2] vdagent-x11: replace WM_NAME related code with GDK
Frediano Ziglio
fziglio at redhat.com
Mon Oct 30 09:19:08 UTC 2017
>
> remove from x11:
> - vdagent_x11_get_wm_name()
> - vdagent_x11_has_icons_on_desktop()
> - char *new_wm_name
>
> vdagent_x11_has_icons_on_desktop() handling has been
> moved from x11.c to vdagent.c and now uses GDK.
> ---
> src/vdagent/vdagent.c | 46 +++++++++++++++++++++--
> src/vdagent/x11-priv.h | 1 -
> src/vdagent/x11.c | 99
> --------------------------------------------------
> src/vdagent/x11.h | 2 -
> 4 files changed, 42 insertions(+), 106 deletions(-)
>
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index 4710a44..a19a77e 100644
> --- a/src/vdagent/vdagent.c
> +++ b/src/vdagent/vdagent.c
> @@ -37,6 +37,10 @@
> #include <poll.h>
> #include <glib-unix.h>
> #include <gtk/gtk.h>
> +#include <gdk/gdk.h>
> +#ifdef GDK_WINDOWING_X11
> +#include <gdk/gdkx.h>
> +#endif
>
> #include "udscs.h"
> #include "vdagentd-proto.h"
> @@ -51,6 +55,8 @@ typedef struct VDAgent {
> struct udscs_connection *conn;
> GIOChannel *x11_channel;
>
> + gboolean has_icons_on_desktop;
> +
> GMainLoop *loop;
> } VDAgent;
>
> @@ -92,6 +98,36 @@ static GOptionEntry entries[] = {
> { NULL }
> };
>
> +static gboolean vdagent_has_icons_on_desktop()
> +{
> +#ifdef GDK_WINDOWING_X11
> + const char * const wms_with_icons[] = {
> + "Metacity", /* GNOME-2 or GNOME-3 fallback */
> + "Xfwm4", /* Xfce */
> + "Marco", /* Mate */
> + };
> +
> + GdkDisplay *display = gdk_display_get_default();
> + if (!GDK_IS_X11_DISPLAY(display))
> + return FALSE;
> +
> + for (int i = 0; i < 10; i++) {
> + const char *wm_name = gdk_x11_screen_get_window_manager_name(
> + gdk_display_get_default_screen(display));
> +
> + if (strcmp(wm_name, "unknown")) {
> + for (i = 0; i < G_N_ELEMENTS(wms_with_icons); i++)
> + if (!strcmp(wm_name, wms_with_icons[i]))
> + return TRUE;
> + return FALSE;
> + }
> +
> + usleep(100000);
> + }
> +#endif
> + return FALSE;
> +}
> +
> /**
> * xfer_get_download_directory
> *
> @@ -109,7 +145,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(agent->has_icons_on_desktop ?
> G_USER_DIRECTORY_DESKTOP :
> G_USER_DIRECTORY_DOWNLOAD);
> }
> @@ -123,6 +159,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 +174,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 ? !agent->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);
> }
>
> @@ -372,6 +408,8 @@ static gboolean vdagent_init_async_cb(gpointer user_data)
> x11_io_channel_cb,
> agent);
>
> + agent->has_icons_on_desktop = vdagent_has_icons_on_desktop();
> +
> if (!vdagent_init_file_xfer(agent))
> syslog(LOG_WARNING, "File transfer is disabled");
>
> 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 6e47ea1..7387a00 100644
> --- a/src/vdagent/x11.c
> +++ b/src/vdagent/x11.c
> @@ -37,7 +37,6 @@
> #include <string.h>
> #include <syslog.h>
> #include <assert.h>
> -#include <unistd.h>
> #include <X11/Xatom.h>
> #include <X11/Xlib.h>
> #include <X11/extensions/Xfixes.h>
> @@ -111,68 +110,6 @@ int vdagent_x11_restore_error_handler(struct vdagent_x11
> *x11)
> return error;
> }
>
> -static void vdagent_x11_get_wm_name(struct vdagent_x11 *x11)
> -{
> - Atom type_ret;
> - int format_ret;
> - unsigned long len, remain;
> - unsigned char *data = NULL;
> - Window sup_window = None;
> -
> - /* XGetWindowProperty can throw a BadWindow error. One way we can
> trigger
> - this is when the display-manager (ie gdm) has set, and not cleared
> the
> - _NET_SUPPORTING_WM_CHECK property, and the window manager running in
> - the user session has not yet updated it to point to its window, so
> its
> - pointing to a nonexistent window. */
> - vdagent_x11_set_error_handler(x11,
> vdagent_x11_ignore_bad_window_handler);
> -
> - /* Get the window manager SUPPORTING_WM_CHECK window */
> - if (XGetWindowProperty(x11->display, x11->root_window[0],
> - XInternAtom(x11->display, "_NET_SUPPORTING_WM_CHECK", False), 0,
> - LONG_MAX, False, XA_WINDOW, &type_ret, &format_ret, &len,
> - &remain, &data) == Success) {
> - if (type_ret == XA_WINDOW)
> - sup_window = *((Window *)data);
> - XFree(data);
> - }
> - if (sup_window == None &&
> - XGetWindowProperty(x11->display, x11->root_window[0],
> - XInternAtom(x11->display, "_WIN_SUPPORTING_WM_CHECK", False), 0,
> - LONG_MAX, False, XA_CARDINAL, &type_ret, &format_ret, &len,
> - &remain, &data) == Success) {
> - if (type_ret == XA_CARDINAL)
> - sup_window = *((Window *)data);
> - XFree(data);
> - }
> - /* So that we can get the net_wm_name */
> - if (sup_window != None) {
> - Atom utf8 = XInternAtom(x11->display, "UTF8_STRING", False);
> - if (XGetWindowProperty(x11->display, sup_window,
> - XInternAtom(x11->display, "_NET_WM_NAME", False), 0,
> - LONG_MAX, False, utf8, &type_ret, &format_ret, &len,
> - &remain, &data) == Success) {
> - if (type_ret == utf8) {
> - x11->net_wm_name =
> - g_strndup((char *)data, (format_ret / 8) * len);
> - }
> - XFree(data);
> - }
> - if (x11->net_wm_name == NULL &&
> - XGetWindowProperty(x11->display, sup_window,
> - XInternAtom(x11->display, "_NET_WM_NAME", False), 0,
> - LONG_MAX, False, XA_STRING, &type_ret, &format_ret, &len,
> - &remain, &data) == Success) {
> - if (type_ret == XA_STRING) {
> - x11->net_wm_name =
> - g_strndup((char *)data, (format_ret / 8) * len);
> - }
> - XFree(data);
> - }
> - }
> -
> - vdagent_x11_restore_error_handler(x11);
> -}
> -
> struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
> int debug, int sync)
> {
> @@ -275,18 +212,6 @@ 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;
> - vdagent_x11_get_wm_name(x11);
> - while (x11->net_wm_name == NULL && --i > 0) {
> - usleep(100000);
> - vdagent_x11_get_wm_name(x11);
> - }
> - if (x11->debug && x11->net_wm_name)
> - syslog(LOG_DEBUG, "net_wm_name: \"%s\", has icons: %d",
> - x11->net_wm_name, vdagent_x11_has_icons_on_desktop(x11));
> -
> /* Flush output buffers and consume any pending events */
> vdagent_x11_do_read(x11);
>
> @@ -308,7 +233,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);
> }
> @@ -1331,26 +1255,3 @@ void vdagent_x11_client_disconnected(struct
> vdagent_x11 *x11)
> vdagent_x11_clipboard_release(x11, sel);
> }
> }
> -
> -/* Function used to determine the default location to save file-xfers,
> - xdg desktop dir or xdg download dir. We err on the safe side and use a
> - 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)
> -{
> - const char * const wms_with_icons_on_desktop[] = {
> - "Metacity", /* GNOME-2 or GNOME-3 fallback */
> - "Xfwm4", /* Xfce */
> - "Marco", /* Mate */
> - NULL
> - };
> - int i;
> -
> - if (x11->net_wm_name)
> - for (i = 0; wms_with_icons_on_desktop[i]; i++)
> - if (!strcmp(x11->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..2060655 100644
> --- a/src/vdagent/x11.h
> +++ b/src/vdagent/x11.h
> @@ -48,6 +48,4 @@ 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);
> -
> #endif
Looks like this patch is doing too much thing:
- move code from x11.c to vdagent.c, why?
- change cache from name to boolean, seems fine;
- use GDK instead of direct X11 calls.
I personally like code dealing with X11 being in x11.c instead of vdagent.c.
Frediano
More information about the Spice-devel
mailing list