Hi, On Sun, Jan 21, 2018 at 09:03:14PM +0100, Jakub Janků wrote: > From: Jakub Janků <jjanku@xxxxxxxxxx> > > Place the code that handles clipboard > into a separate file - clipboard.c > --- > Makefile.am | 2 + > src/vdagent/clipboard.c | 401 ++++++++++++++++++++++++++++++++++++++++++++++++ > src/vdagent/clipboard.h | 42 +++++ > src/vdagent/vdagent.c | 31 +++- > 4 files changed, 471 insertions(+), 5 deletions(-) > create mode 100644 src/vdagent/clipboard.c > create mode 100644 src/vdagent/clipboard.h > > diff --git a/Makefile.am b/Makefile.am > index c4bd3dd..88550c6 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -33,6 +33,8 @@ src_spice_vdagent_SOURCES = \ > $(common_sources) \ > src/vdagent/audio.c \ > src/vdagent/audio.h \ > + src/vdagent/clipboard.c \ > + src/vdagent/clipboard.h \ > src/vdagent/file-xfers.c \ > src/vdagent/file-xfers.h \ > src/vdagent/x11-priv.h \ > diff --git a/src/vdagent/clipboard.c b/src/vdagent/clipboard.c > new file mode 100644 > index 0000000..657a6b4 > --- /dev/null > +++ b/src/vdagent/clipboard.c > @@ -0,0 +1,401 @@ > +/* clipboard.c - vdagent clipboard handling code > + > + Copyright 2017 Red Hat, Inc. > + > + This program is free software: you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation, either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see <http://www.gnu.org/licenses/>. > +*/ > + > +#ifdef HAVE_CONFIG_H > +# include <config.h> > +#endif > + > +#include <gtk/gtk.h> > +#include <syslog.h> > + > +#include "vdagentd-proto.h" > +#include "spice/vd_agent.h" > +#include "udscs.h" > +#include "clipboard.h" > + > +/* 2 selections supported - _SELECTION_CLIPBOARD = 0, _SELECTION_PRIMARY = 1 */ > +#define SELECTION_COUNT (VD_AGENT_CLIPBOARD_SELECTION_PRIMARY + 1) > +#define TYPE_COUNT (VD_AGENT_CLIPBOARD_IMAGE_JPG + 1) > + > +#define sel_from_clip(clipboard) \ > + GPOINTER_TO_UINT(g_object_get_data(G_OBJECT(clipboard), "vdagent-selection")) > + > +enum { > + OWNER_NONE, > + OWNER_GUEST, > + OWNER_CLIENT > +}; > + > +typedef struct { > + GMainLoop *loop; > + GtkSelectionData *sel_data; > +} DataRetrieval; > + > +struct VDAgentClipboards { > + struct udscs_connection *conn; > + > + GtkClipboard *clipboard[SELECTION_COUNT]; > + guint owner[SELECTION_COUNT]; > + > + GList *data_retrievals[SELECTION_COUNT]; > + GList *data_requests[SELECTION_COUNT]; I think we could use something more clear instead of 'data_retrievals' and 'data_requests' but I'm terrible with names myself so I suggest 'pending_set_request' and 'pending_get_requests' or just 'set_requests' and 'get_requests'. A comment would help too, like /* Client -> Guest */ and vice versa. > + gpointer *last_targets_req[SELECTION_COUNT]; > + > + GdkAtom targets[SELECTION_COUNT][TYPE_COUNT]; Maybe wrap them in inner struct { ... } selection[SELECTION_TYPE] ? e.g. - c->data_requests[sel] = ... + c->selection[id].data_request = ... > +}; > + > +static const struct { > + guint type; > + const gchar *atom_name; > +} atom2agent[] = { > + {VD_AGENT_CLIPBOARD_UTF8_TEXT, "UTF8_STRING"}, > + {VD_AGENT_CLIPBOARD_UTF8_TEXT, "text/plain;charset=utf-8"}, > + {VD_AGENT_CLIPBOARD_UTF8_TEXT, "STRING"}, > + {VD_AGENT_CLIPBOARD_UTF8_TEXT, "TEXT"}, > + {VD_AGENT_CLIPBOARD_UTF8_TEXT, "text/plain"}, > + {VD_AGENT_CLIPBOARD_IMAGE_PNG, "image/png"}, > + {VD_AGENT_CLIPBOARD_IMAGE_BMP, "image/bmp"}, > + {VD_AGENT_CLIPBOARD_IMAGE_BMP, "image/x-bmp"}, > + {VD_AGENT_CLIPBOARD_IMAGE_BMP, "image/x-MS-bmp"}, > + {VD_AGENT_CLIPBOARD_IMAGE_BMP, "image/x-win-bitmap"}, > + {VD_AGENT_CLIPBOARD_IMAGE_TIFF,"image/tiff"}, > + {VD_AGENT_CLIPBOARD_IMAGE_JPG, "image/jpeg"}, > +}; > + > +static guint get_type_from_atom(GdkAtom atom) > +{ > + int i; > + gchar *name = gdk_atom_name(atom); > + for (i = 0; i < G_N_ELEMENTS(atom2agent); i++) > + if (!g_ascii_strcasecmp(name, atom2agent[i].atom_name)) > + break; > + g_free(name); > + return i < G_N_ELEMENTS(atom2agent) ? atom2agent[i].type > + : VD_AGENT_CLIPBOARD_NONE; > +} > + > +/* gtk_clipboard_request_(, callback, user_data) cannot be cancelled. Instead, > + gpointer *ref = get_weak_ref(VDAgentClipboards *) is passed to the callback. > + The callback returns if free_weak_ref(ref) == NULL, which can be done > + by setting *ref = NULL. This substitutes cancellation of the request. > + */ > +static gpointer *get_weak_ref(gpointer data) > +{ > + gpointer *ref = g_new(gpointer, 1); > + *ref = data; > + return ref; > +} So, this is similar to GWeakRef but we are not using a GObject at all. I might be missing something but why not an actual structure for the pending request? I think it would be easier to understand and track what is going. At least we should have some functions with the clear meaning around this, like weak_ref_cancel(gpointer ref) { /* check and set to NULL */ } ... but I do think something around GCancellable would be nicer or even bool is_cancelled; in a custom struct. > +static gpointer free_weak_ref(gpointer *ref) > +{ It might be better to add: if (ref == NULL) return NULL; > + gpointer data = *ref; That way you can pass NULL to the function and without trying to access it. > + g_free(ref); > + return data; > +} > + > +static void clipboard_new_owner(VDAgentClipboards *c, guint sel, guint new_owner) > +{ > + GList *l; > + /* let the other apps know no data is coming */ > + for (l = c->data_retrievals[sel]; l != NULL; l= l->next) { > + DataRetrieval *retrv = l->data; > + g_main_loop_quit(retrv->loop); > + } > + g_clear_pointer(&c->data_retrievals[sel], g_list_free); > + > + /* respond to pending client's data requests */ > + for (l = c->data_requests[sel]; l != NULL; l = l->next) { > + gpointer *ref = l->data; > + *ref = NULL; > + if (c->conn) > + udscs_write(c->conn, VDAGENTD_CLIPBOARD_DATA, > + sel, VD_AGENT_CLIPBOARD_NONE, NULL, 0); > + } > + g_clear_pointer(&c->data_requests[sel], g_list_free); > + > + c->owner[sel] = new_owner; > +} > + > +static void clipboard_targets_received_cb(GtkClipboard *clipboard, > + GdkAtom *atoms, > + gint n_atoms, > + gpointer user_data) > +{ > + VDAgentClipboards *c = free_weak_ref(user_data); > + guint32 types[G_N_ELEMENTS(atom2agent)]; > + guint sel, type, n_types = 0, a; > + > + if (c == NULL) /* request was cancelled */ > + return; > + > + sel = sel_from_clip(clipboard); > + c->last_targets_req[sel] = NULL; > + > + if (atoms == NULL) > + return; > + > + for (type = 0; type < TYPE_COUNT; type++) > + c->targets[sel][type] = GDK_NONE; > + > + for (a = 0; a < n_atoms; a++) { > + type = get_type_from_atom(atoms[a]); > + if (type == VD_AGENT_CLIPBOARD_NONE || c->targets[sel][type] != GDK_NONE) > + continue; > + > + c->targets[sel][type] = atoms[a]; > + types[n_types] = type; > + n_types++; > + } > + > + if (n_types == 0) { > + syslog(LOG_WARNING, "%s: sel=%u: no target supported", __func__, sel); > + return; > + } > + > + clipboard_new_owner(c, sel, OWNER_GUEST); > + > + udscs_write(c->conn, VDAGENTD_CLIPBOARD_GRAB, sel, 0, > + (guint8 *)types, n_types * sizeof(guint32)); > +} > + > +static void clipboard_owner_change_cb(GtkClipboard *clipboard, > + GdkEventOwnerChange *event, > + gpointer user_data) > +{ > + VDAgentClipboards *c = user_data; > + guint sel = sel_from_clip(clipboard); > + > + /* if the event was caused by gtk_clipboard_set_with_data(), ignore it */ > + if (c->owner[sel] == OWNER_CLIENT) > + return; > + > + if (c->owner[sel] == OWNER_GUEST) { > + clipboard_new_owner(c, sel, OWNER_NONE); > + udscs_write(c->conn, VDAGENTD_CLIPBOARD_RELEASE, sel, 0, NULL, 0); > + } > + > + if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) > + return; > + > + /* if there's a pending request for clipboard targets, cancel it */ > + if (c->last_targets_req[sel]) > + *(c->last_targets_req[sel]) = NULL; > + > + c->last_targets_req[sel] = get_weak_ref(c); > + gtk_clipboard_request_targets(clipboard, clipboard_targets_received_cb, > + c->last_targets_req[sel]); > +} > + > +static void clipboard_contents_received_cb(GtkClipboard *clipboard, > + GtkSelectionData *sel_data, > + gpointer user_data) > +{ > + guint sel, type, target; > + VDAgentClipboards *c = free_weak_ref(user_data); > + if (c == NULL) /* request was cancelled */ > + return; > + > + sel = sel_from_clip(clipboard); > + c->data_requests[sel] = g_list_remove(c->data_requests[sel], user_data); > + > + type = get_type_from_atom(gtk_selection_data_get_data_type(sel_data)); > + target = get_type_from_atom(gtk_selection_data_get_target(sel_data)); > + > + if (type == target) { > + udscs_write(c->conn, VDAGENTD_CLIPBOARD_DATA, sel, type, > + gtk_selection_data_get_data(sel_data), > + gtk_selection_data_get_length(sel_data)); > + } else { > + syslog(LOG_WARNING, "%s: sel=%u: expected type %u, recieved %u, " > + "skipping", __func__, sel, target, type); > + udscs_write(c->conn, VDAGENTD_CLIPBOARD_DATA, sel, > + VD_AGENT_CLIPBOARD_NONE, NULL, 0); > + } > +} > + > +static void clipboard_get_cb(GtkClipboard *clipboard, > + GtkSelectionData *sel_data, > + guint info, > + gpointer user_data) > +{ > + DataRetrieval retrv; > + VDAgentClipboards *c = user_data; > + guint sel, type; > + > + sel = sel_from_clip(clipboard); > + g_return_if_fail(c->owner[sel] == OWNER_CLIENT); > + > + type = get_type_from_atom(gtk_selection_data_get_target(sel_data)); > + g_return_if_fail(type != VD_AGENT_CLIPBOARD_NONE); > + > + retrv.sel_data = sel_data; > + retrv.loop = g_main_loop_new(NULL, FALSE); > + c->data_retrievals[sel] = g_list_prepend(c->data_retrievals[sel], &retrv); > + > + udscs_write(c->conn, VDAGENTD_CLIPBOARD_REQUEST, sel, type, NULL, 0); > + > +G_GNUC_BEGIN_IGNORE_DEPRECATIONS > + gdk_threads_leave(); > + g_main_loop_run(retrv.loop); Wouldn't this block the agent while we don't receive input from the client? I'm mostly worried in case of errors (client disconnect, data not sent, etc). > + gdk_threads_enter(); > +G_GNUC_END_IGNORE_DEPRECATIONS > + > + g_main_loop_unref(retrv.loop); > +} > + > +static void clipboard_clear_cb(GtkClipboard *clipboard, gpointer user_data) > +{ > + VDAgentClipboards *c = user_data; > + clipboard_new_owner(c, sel_from_clip(clipboard), OWNER_NONE); > +} > + > +void vdagent_clipboard_grab(VDAgentClipboards *c, guint sel, > + guint32 *types, guint n_types) > +{ > + GtkTargetEntry targets[G_N_ELEMENTS(atom2agent)]; > + guint n_targets = 0, i, t; > + > + g_return_if_fail(sel < SELECTION_COUNT); > + > + for (i = 0; i < G_N_ELEMENTS(atom2agent); i++) > + for (t = 0; t < n_types; t++) > + if (atom2agent[i].type == types[t]) { > + targets[n_targets].target = (gchar *)atom2agent[i].atom_name; > + n_targets++; > + break; > + } > + > + if (n_targets == 0) { > + syslog(LOG_WARNING, "%s: sel=%u: no type supported", __func__, sel); > + return; > + } > + > + if (gtk_clipboard_set_with_data(c->clipboard[sel], targets, n_targets, > + clipboard_get_cb, clipboard_clear_cb, c)) > + clipboard_new_owner(c, sel, OWNER_CLIENT); > + else { > + syslog(LOG_ERR, "%s: sel=%u: clipboard grab failed", __func__, sel); > + clipboard_new_owner(c, sel, OWNER_NONE); > + } > +} > + > +void vdagent_clipboard_data(VDAgentClipboards *c, guint sel, > + guint type, const guchar *data, guint size) > +{ > + g_return_if_fail(sel < SELECTION_COUNT); > + > + DataRetrieval *retrv; > + GList *l; > + for (l = c->data_retrievals[sel]; l != NULL; l = l->next) { > + retrv = l->data; > + if (get_type_from_atom(gtk_selection_data_get_target(retrv->sel_data)) == type) > + break; > + } > + if (l == NULL) { > + syslog(LOG_WARNING, "%s: sel=%u: no corresponding request found for " > + "type=%u, skipping", __func__, sel, type); > + return; g_main_loop_quit() here? > + } > + c->data_retrievals[sel] = g_list_delete_link(c->data_retrievals[sel], l); > + > + gtk_selection_data_set(retrv->sel_data, > + gtk_selection_data_get_target(retrv->sel_data), > + 8, data, size); > + > + g_main_loop_quit(retrv->loop); > +} > + > +void vdagent_clipboard_release(VDAgentClipboards *c, guint sel) > +{ > + g_return_if_fail(sel < SELECTION_COUNT); > + if (c->owner[sel] != OWNER_CLIENT) > + return; > + > + clipboard_new_owner(c, sel, OWNER_NONE); > + gtk_clipboard_clear(c->clipboard[sel]); > +} > + > +void vdagent_clipboard_release_all(VDAgentClipboards *c) > +{ > + guint sel, owner; > + > + for (sel = 0; sel < SELECTION_COUNT; sel++) { > + owner = c->owner[sel]; > + clipboard_new_owner(c, sel, OWNER_NONE); > + if (owner == OWNER_CLIENT) > + gtk_clipboard_clear(c->clipboard[sel]); > + else if (owner == OWNER_GUEST && c->conn) > + udscs_write(c->conn, VDAGENTD_CLIPBOARD_RELEASE, sel, 0, NULL, 0); > + } > +} > + > +void vdagent_clipboard_request(VDAgentClipboards *c, guint sel, guint type) > +{ > + if (sel >= SELECTION_COUNT || c->owner[sel] != OWNER_GUEST) I'm all for logging debug or warnings in such situations. Reviewed-by: Victor Toso <victortoso@xxxxxxxxxx> toso > + goto err; > + if (type >= TYPE_COUNT || c->targets[sel][type] == GDK_NONE) { > + syslog(LOG_WARNING, "%s: sel=%d: unadvertised data type requested", > + __func__, sel); > + goto err; > + } > + > + gpointer *ref = get_weak_ref(c); > + c->data_requests[sel] = g_list_prepend(c->data_requests[sel], ref); > + gtk_clipboard_request_contents(c->clipboard[sel], c->targets[sel][type], > + clipboard_contents_received_cb, ref); > + return; > +err: > + udscs_write(c->conn, VDAGENTD_CLIPBOARD_DATA, sel, > + VD_AGENT_CLIPBOARD_NONE, NULL, 0); > +} > + > +VDAgentClipboards *vdagent_clipboard_init(struct udscs_connection *conn) > +{ > + const GdkAtom sel_atom[SELECTION_COUNT] = { > + GDK_SELECTION_CLIPBOARD, /* VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD */ > + GDK_SELECTION_PRIMARY, /* VD_AGENT_CLIPBOARD_SELECTION_PRIMARY */ > + }; > + > + VDAgentClipboards *c; > + c = g_new0(VDAgentClipboards, 1); > + c->conn = conn; > + > + for (guint sel = 0; sel < SELECTION_COUNT; sel++) { > + c->clipboard[sel] = gtk_clipboard_get(sel_atom[sel]); > + /* enables the use of sel_from_clipboard(clipboard) macro */ > + g_object_set_data(G_OBJECT(c->clipboard[sel]), "vdagent-selection", > + GUINT_TO_POINTER(sel)); > + g_signal_connect(G_OBJECT(c->clipboard[sel]), "owner-change", > + G_CALLBACK(clipboard_owner_change_cb), c); > + } > + > + return c; > +} > + > +void vdagent_clipboard_finalize(VDAgentClipboards *c, > + struct udscs_connection *conn) > +{ > + for (guint sel = 0; sel < SELECTION_COUNT; sel++) > + g_signal_handlers_disconnect_by_func(c->clipboard[sel], > + G_CALLBACK(clipboard_owner_change_cb), c); > + > + c->conn = conn; > + vdagent_clipboard_release_all(c); > + > + g_free(c); > +} > diff --git a/src/vdagent/clipboard.h b/src/vdagent/clipboard.h > new file mode 100644 > index 0000000..f02d8a4 > --- /dev/null > +++ b/src/vdagent/clipboard.h > @@ -0,0 +1,42 @@ > +/* clipboard.h - vdagent clipboard handling header > + > + Copyright 2017 Red Hat, Inc. > + > + This program is free software: you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation, either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see <http://www.gnu.org/licenses/>. > +*/ > + > +#ifndef __VDAGENT_CLIPBOARD_H > +#define __VDAGENT_CLIPBOARD_H > + > +#include "udscs.h" > + > +typedef struct VDAgentClipboards VDAgentClipboards; > + > +VDAgentClipboards *vdagent_clipboard_init(struct udscs_connection *conn); > +void vdagent_clipboard_finalize(VDAgentClipboards *c, > + struct udscs_connection *conn); > + > +void vdagent_clipboard_request(VDAgentClipboards *c, guint sel, guint type); > + > +void vdagent_clipboard_release(VDAgentClipboards *c, guint sel); > + > +void vdagent_clipboard_release_all(VDAgentClipboards *c); > + > +void vdagent_clipboard_data(VDAgentClipboards *c, guint sel, > + guint type, const guchar *data, guint size); > + > +void vdagent_clipboard_grab(VDAgentClipboards *c, guint sel, > + guint32 *types, guint n_types); > + > +#endif > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c > index 92ffcf3..81b7a5a 100644 > --- a/src/vdagent/vdagent.c > +++ b/src/vdagent/vdagent.c > @@ -44,8 +44,10 @@ > #include "audio.h" > #include "x11.h" > #include "file-xfers.h" > +#include "clipboard.h" > > typedef struct VDAgent { > + VDAgentClipboards *clipboards; > struct vdagent_x11 *x11; > struct vdagent_file_xfers *xfers; > struct udscs_connection *conn; > @@ -154,6 +156,17 @@ static gboolean vdagent_finalize_file_xfer(VDAgent *agent) > return TRUE; > } > > +static void vdagent_quit_loop(VDAgent *agent) > +{ > + /* other GMainLoop(s) might be running, quit them before agent->loop */ > + if (agent->clipboards) { > + vdagent_clipboard_finalize(agent->clipboards, agent->conn); > + agent->clipboards = NULL; > + } > + if (agent->loop) > + g_main_loop_quit(agent->loop); > +} > + > static void daemon_read_complete(struct udscs_connection **connp, > struct udscs_message_header *header, uint8_t *data) > { > @@ -164,18 +177,24 @@ static void daemon_read_complete(struct udscs_connection **connp, > vdagent_x11_set_monitor_config(agent->x11, (VDAgentMonitorsConfig *)data, 0); > break; > case VDAGENTD_CLIPBOARD_REQUEST: > + vdagent_clipboard_request(agent->clipboards, header->arg1, header->arg2); > break; > case VDAGENTD_CLIPBOARD_GRAB: > + vdagent_clipboard_grab(agent->clipboards, header->arg1, > + (guint32 *)data, header->size / sizeof(guint32)); > break; > case VDAGENTD_CLIPBOARD_DATA: > + vdagent_clipboard_data(agent->clipboards, header->arg1, header->arg2, > + data, header->size); > break; > case VDAGENTD_CLIPBOARD_RELEASE: > + vdagent_clipboard_release(agent->clipboards, header->arg1); > break; > case VDAGENTD_VERSION: > if (strcmp((char *)data, VERSION) != 0) { > syslog(LOG_INFO, "vdagentd version mismatch: got %s expected %s", > data, VERSION); > - g_main_loop_quit(agent->loop); > + vdagent_quit_loop(agent); > version_mismatch = 1; > } > break; > @@ -222,6 +241,7 @@ static void daemon_read_complete(struct udscs_connection **connp, > } > break; > case VDAGENTD_CLIENT_DISCONNECTED: > + vdagent_clipboard_release_all(agent->clipboards); > if (vdagent_finalize_file_xfer(agent)) { > vdagent_init_file_xfer(agent); > } > @@ -236,8 +256,7 @@ static void daemon_disconnect_cb(struct udscs_connection *conn) > { > VDAgent *agent = udscs_get_user_data(conn); > agent->conn = NULL; > - if (agent->loop) > - g_main_loop_quit(agent->loop); > + vdagent_quit_loop(agent); > } > > /* When we daemonize, it is useful to have the main process > @@ -309,7 +328,7 @@ gboolean vdagent_signal_handler(gpointer user_data) > { > VDAgent *agent = user_data; > quit = TRUE; > - g_main_loop_quit(agent->loop); > + vdagent_quit_loop(agent); > return G_SOURCE_REMOVE; > } > > @@ -368,6 +387,8 @@ static gboolean vdagent_init_async_cb(gpointer user_data) > if (!vdagent_init_file_xfer(agent)) > syslog(LOG_WARNING, "File transfer is disabled"); > > + agent->clipboards = vdagent_clipboard_init(agent->conn); > + > if (parent_socket != -1) { > if (write(parent_socket, "OK", 2) != 2) > syslog(LOG_WARNING, "Parent already gone."); > @@ -378,7 +399,7 @@ static gboolean vdagent_init_async_cb(gpointer user_data) > return G_SOURCE_REMOVE; > > err_init: > - g_main_loop_quit(agent->loop); > + vdagent_quit_loop(agent); > quit = TRUE; > return G_SOURCE_REMOVE; > } > -- > 2.14.3 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel