[Spice-devel] [PATCH vdagent 2/2] vdagent: handle clipboard using GTK+
Jakub Janku
jjanku at redhat.com
Sun Feb 18 22:44:38 UTC 2018
Hi Victor,
sorry for the delayed reply.
On Mon, Feb 12, 2018 at 2:11 PM, Victor Toso <victortoso at redhat.com> wrote:
> Hi,
>
> On Sun, Jan 21, 2018 at 09:03:14PM +0100, Jakub Janků wrote:
>> From: Jakub Janků <jjanku at redhat.com>
>>
>> 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'.
I agree, the names are far from ideal and rather confusing, but
'set_requests' and 'get_requests' aren't making it more clear either
imho - "get request" could stand for a request from the client as well
as a request from an app on the vdagent side. What about
'request_from_client' and 'request_from_app'?
>
> A comment would help too, like /* Client -> Guest */ and vice
> versa.
Definitely, I should have added some comments.
>
>> + 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 = ...
>
Makes sense probably. It might also be handy to typedef something like
DataSelection and use
DataSelection *sel = &c->selection[sel_id];
sel->data_requests = ...
where the struct members are accessed frequently.
>> +};
>> +
>> +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.
>
The get_weak_ref() is only used with callbacks since we need to pass a
reference to VDAgentClipboards. So I'm not sure GCancellable would
help us here.
The struct would need to contain pointer to VDAgentClipboards as well
as the bool is_cancelled, so using something like GWeakRef seemed to
be the easiest solution, although creating a struct might be nicer...
>> +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).
>
>
I'm not sure what you mean by that. "input from the client" = clipboard data?
The g_main_loop_run() call runs a loop inside the clipboard_get_cb() =
it runs a loop inside one iteration of the main loop in vdagent.c
This doesn't block the agent in any way. Apart from that, all loops
started here are quit with owner change (clipboard_new_owner()), so
there's no loop hanging "forever". But we could still add some kind of
timeout using g_timeout_add_seconds().
>> + 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?
We didn't find the matching request, so which loop would you want to
quit here? Unlike the previous X11 implementation, multiple requests
can be pending (I probably should have mentioned that in the commit
message, sorry).
>
>> + }
>> + 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 at redhat.com>
>
> 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
Concerning the naming, I also considered using "Selection" instead of
"Clipboard" (e.g. "vdagent_selection_request" instead of
"vdagent_clipboard_request"). I think this would make more sense when
later intoducing the drag&drop support. What do you think?
Thanks for the review.
Cheers,
Jakub
More information about the Spice-devel
mailing list