On Oct 30, 2017 10:19 AM, "Frediano Ziglio" <fziglio@xxxxxxxxxx> wrote:
>Looks like this patch is doing too much thing:
> 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 = ""> > - 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
- 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.
Hi,
In the future, I'd like to replace all X11 code with GTK (I'm not sure this will be possible everywhere though). So moving this code from x11.c into vdagent.c might be the first step towards removing x11.c entirely. Moving code handling clipboard into clipboard.c and using GTK will follow hopefully soon.
Frediano
Jakub
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel