<div dir="ltr"> <div class="gmail_quote"><div dir="ltr">On Wed, Sep 27, 2017 at 1:02 PM Frediano Ziglio <<a href="mailto:fziglio@redhat.com">fziglio@redhat.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> > Related to ongoing work to use GMainLoop and GTK integration > > Signed-off-by: Victor Toso <<a href="mailto:victortoso@redhat.com" target="_blank">victortoso@redhat.com</a>> > --- > Makefile.am | 3 + > <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> | 1 + > src/vdagent/vdagent.c | 149 > +++++++++++++++++++++++++++++++------------------- > src/vdagent/vdagent.h | 54 ++++++++++++++++++ > 4 files changed, 150 insertions(+), 57 deletions(-) > create mode 100644 src/vdagent/vdagent.h > > diff --git a/Makefile.am b/Makefile.am > index 45f7177..efd058a 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -14,6 +14,7 @@ common_sources = \ > src_spice_vdagent_CFLAGS = \ > $(X_CFLAGS) \ > $(SPICE_CFLAGS) \ > + $(GOBJECT2_CFLAGS) \ > $(GLIB2_CFLAGS) \ > $(ALSA_CFLAGS) \ > -I$(srcdir)/src \ > @@ -23,6 +24,7 @@ src_spice_vdagent_CFLAGS = \ > src_spice_vdagent_LDADD = \ > $(X_LIBS) \ > $(SPICE_LIBS) \ > + $(GOBJECT2_LIBS) \ > $(GLIB2_LIBS) \ > $(ALSA_LIBS) \ > $(NULL) > @@ -38,6 +40,7 @@ src_spice_vdagent_SOURCES = \ > src/vdagent/x11.c \ > src/vdagent/x11.h \ > src/vdagent/vdagent.c \ > + src/vdagent/vdagent.h \ > $(NULL) > > src_spice_vdagentd_CFLAGS = \ > diff --git a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> > index d92b527..12b7d23 100644 > --- a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> > +++ b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> > @@ -86,6 +86,7 @@ AC_ARG_ENABLE([static-uinput], > [enable_static_uinput="no"]) > > PKG_CHECK_MODULES([GLIB2], [glib-2.0 >= 2.34]) > +PKG_CHECK_MODULES(GOBJECT2, [gobject-2.0]) > PKG_CHECK_MODULES(X, [xfixes xrandr >= 1.3 xinerama x11]) > PKG_CHECK_MODULES(SPICE, [spice-protocol >= 0.12.13]) > PKG_CHECK_MODULES(ALSA, [alsa >= 1.0.22]) > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c > index 0b6df3f..cdbda8b 100644 > --- a/src/vdagent/vdagent.c > +++ b/src/vdagent/vdagent.c > @@ -37,16 +37,13 @@ > #include <glib.h> > #include <poll.h> > > -#include "udscs.h" > #include "vdagentd-proto.h" > #include "vdagentd-proto-strings.h" > #include "audio.h" > -#include "x11.h" > -#include "file-xfers.h" > +#include "vdagent.h" > + > +G_DEFINE_TYPE (SpiceVDAgent, spice_vdagent, G_TYPE_OBJECT); > Lot of functions use already the vdagent_ prefix. Would not easier to call the class just VDAgent ? </blockquote><div>Victor used "SessionAgent" in the earlier version, what about that?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > -static struct vdagent_x11 *x11 = NULL; > -static struct vdagent_file_xfers *vdagent_file_xfers = NULL; > -static struct udscs_connection *client = NULL; > static int quit = 0; > static int version_mismatch = 0; > > @@ -90,12 +87,12 @@ static GOptionEntry entries[] = { > * Return path where transferred files should be stored. > * Returned path should not be freed or modified. > **/ > -static const gchar *xfer_get_download_directory(void) > +static const gchar *xfer_get_download_directory(SpiceVDAgent *agent) > { > if (fx_dir != NULL) > return fx_dir; > > - if (vdagent_x11_has_icons_on_desktop(x11)) > + if (vdagent_x11_has_icons_on_desktop(agent->x11)) > return g_get_user_special_dir(G_USER_DIRECTORY_DESKTOP); > > return g_get_user_special_dir(G_USER_DIRECTORY_DOWNLOAD); > @@ -105,61 +102,63 @@ static const gchar *xfer_get_download_directory(void) > * vdagent_init_file_xfer > * > * Initialize handler for file xfer, > - * return TRUE on success (vdagent_file_xfers is not NULL). > + * return TRUE on success (agent->xfers is not NULL). > **/ > -static gboolean vdagent_init_file_xfer(void) > +static gboolean vdagent_init_file_xfer(SpiceVDAgent *agent) > { > const gchar *xfer_dir; > > - if (vdagent_file_xfers != NULL) { > + if (agent->xfers != NULL) { > syslog(LOG_DEBUG, "File-xfer already initialized"); > return TRUE; > } > > - xfer_dir = xfer_get_download_directory(); > + xfer_dir = xfer_get_download_directory(agent); > if (xfer_dir == NULL) { > syslog(LOG_WARNING, > "warning could not get file xfer save dir, " > "file transfers will be disabled"); > - vdagent_file_xfers = NULL; > return FALSE; > } > > - vdagent_file_xfers = vdagent_file_xfers_create(client, xfer_dir, > - fx_open_dir, debug); > - return (vdagent_file_xfers != NULL); > + agent->xfers = vdagent_file_xfers_create(agent->conn, xfer_dir, > + fx_open_dir, debug); > + return (agent->xfers != NULL); > } > > -static gboolean vdagent_finalize_file_xfer(void) > +static gboolean vdagent_finalize_file_xfer(SpiceVDAgent *agent) > { > - if (vdagent_file_xfers == NULL) > + if (agent->xfers == NULL) > return FALSE; > > - g_clear_pointer(&vdagent_file_xfers, vdagent_file_xfers_destroy); > + g_clear_pointer(&agent->xfers, vdagent_file_xfers_destroy); > return TRUE; > } > > static void daemon_read_complete(struct udscs_connection **connp, > struct udscs_message_header *header, uint8_t *data, > - G_GNUC_UNUSED void *user_data) > + void *user_data) > { > + g_return_if_fail(SPICE_IS_VDAGENT(user_data)); > + SpiceVDAgent *agent = user_data; > + > switch (header->type) { > case VDAGENTD_MONITORS_CONFIG: > - vdagent_x11_set_monitor_config(x11, (VDAgentMonitorsConfig *)data, > 0); > + vdagent_x11_set_monitor_config(agent->x11, (VDAgentMonitorsConfig > *)data, 0); > break; > case VDAGENTD_CLIPBOARD_REQUEST: > - vdagent_x11_clipboard_request(x11, header->arg1, header->arg2); > + vdagent_x11_clipboard_request(agent->x11, header->arg1, > header->arg2); > break; > case VDAGENTD_CLIPBOARD_GRAB: > - vdagent_x11_clipboard_grab(x11, header->arg1, (uint32_t *)data, > + vdagent_x11_clipboard_grab(agent->x11, header->arg1, (uint32_t > *)data, > header->size / sizeof(uint32_t)); > break; > case VDAGENTD_CLIPBOARD_DATA: > - vdagent_x11_clipboard_data(x11, header->arg1, header->arg2, > + vdagent_x11_clipboard_data(agent->x11, header->arg1, header->arg2, > data, header->size); > break; > case VDAGENTD_CLIPBOARD_RELEASE: > - vdagent_x11_clipboard_release(x11, header->arg1); > + vdagent_x11_clipboard_release(agent->x11, header->arg1); > break; > case VDAGENTD_VERSION: > if (strcmp((char *)data, VERSION) != 0) { > @@ -170,8 +169,8 @@ static void daemon_read_complete(struct udscs_connection > **connp, > } > break; > case VDAGENTD_FILE_XFER_START: > - if (vdagent_file_xfers != NULL) { > - vdagent_file_xfers_start(vdagent_file_xfers, > + if (agent->xfers != NULL) { > + vdagent_file_xfers_start(agent->xfers, > (VDAgentFileXferStartMessage *)data); > } else { > vdagent_file_xfers_error_disabled(*connp, > @@ -179,8 +178,8 @@ static void daemon_read_complete(struct udscs_connection > **connp, > } > break; > case VDAGENTD_FILE_XFER_STATUS: > - if (vdagent_file_xfers != NULL) { > - vdagent_file_xfers_status(vdagent_file_xfers, > + if (agent->xfers != NULL) { > + vdagent_file_xfers_status(agent->xfers, > (VDAgentFileXferStatusMessage *)data); > } else { > vdagent_file_xfers_error_disabled(*connp, > @@ -191,7 +190,7 @@ static void daemon_read_complete(struct udscs_connection > **connp, > if (debug) > syslog(LOG_DEBUG, "Disabling file-xfers"); > > - vdagent_finalize_file_xfer(); > + vdagent_finalize_file_xfer(agent); > break; > case VDAGENTD_AUDIO_VOLUME_SYNC: { > VDAgentAudioVolumeSync *avs = (VDAgentAudioVolumeSync *)data; > @@ -203,8 +202,8 @@ static void daemon_read_complete(struct udscs_connection > **connp, > break; > } > case VDAGENTD_FILE_XFER_DATA: > - if (vdagent_file_xfers != NULL) { > - vdagent_file_xfers_data(vdagent_file_xfers, > + if (agent->xfers != NULL) { > + vdagent_file_xfers_data(agent->xfers, > (VDAgentFileXferDataMessage *)data); > } else { > vdagent_file_xfers_error_disabled(*connp, > @@ -212,9 +211,9 @@ static void daemon_read_complete(struct udscs_connection > **connp, > } > break; > case VDAGENTD_CLIENT_DISCONNECTED: > - vdagent_x11_client_disconnected(x11); > - if (vdagent_finalize_file_xfer()) { > - vdagent_init_file_xfer(); > + vdagent_x11_client_disconnected(agent->x11); > + if (vdagent_finalize_file_xfer(agent)) { > + vdagent_init_file_xfer(agent); > } > break; > default: > @@ -223,12 +222,12 @@ static void daemon_read_complete(struct > udscs_connection **connp, > } > } > > -static struct udscs_connection *client_setup_sync(void) > +static struct udscs_connection *client_setup_sync(SpiceVDAgent *agent) > { > struct udscs_connection *conn = NULL; > > while (!quit) { > - conn = udscs_connect(vdagentd_socket, daemon_read_complete, NULL, > + conn = udscs_connect(vdagentd_socket, daemon_read_complete, agent, > NULL, vdagentd_messages, VDAGENTD_NO_MESSAGES, > debug); > if (conn || !do_daemonize || quit) { > @@ -299,6 +298,49 @@ static int file_test(const char *path) > return stat(path, &buffer); > } > > +static void spice_vdagent_init(SpiceVDAgent *self) > +{ > +} > + > +static void spice_vdagent_finalize(GObject *gobject) > +{ > + SpiceVDAgent *self = SPICE_VDAGENT(gobject); > + vdagent_finalize_file_xfer(self); > + vdagent_x11_destroy(self->x11, self->conn == NULL); > + > + if (self->conn != NULL) { > + udscs_destroy_connection(&self->conn); > + self->conn = NULL; > + } > + g_clear_pointer here? </blockquote><div> </div><div>Sure. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + if (G_OBJECT_CLASS(spice_vdagent_parent_class)->finalize) this if is not necessary, see <a href="https://developer.gnome.org/gobject/stable/howto-gobject-destruction.html" rel="noreferrer" target="_blank">https://developer.gnome.org/gobject/stable/howto-gobject-destruction.html</a> </blockquote><div>OK.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + G_OBJECT_CLASS(spice_vdagent_parent_class)->finalize(gobject); > +} > + > +static void spice_vdagent_class_init(SpiceVDAgentClass *klass) > +{ > + GObjectClass *gobject_class = G_OBJECT_CLASS(klass); > + gobject_class->finalize = spice_vdagent_finalize; > +} > + > +static gboolean spice_vdagent_init_sync(SpiceVDAgent *agent) > +{ > + g_return_val_if_fail(SPICE_IS_VDAGENT(agent), FALSE); > + I don't think this check is necessary. > + agent->conn = client_setup_sync(agent); > + if (agent->conn == NULL) > + return FALSE; > + > + agent->x11 = vdagent_x11_create(agent->conn, debug, x11_sync); > + if (agent->x11 == NULL) > + return FALSE; > + > + if (!vdagent_init_file_xfer(agent)) > + syslog(LOG_WARNING, "File transfer is disabled"); > + > + return TRUE; > +} > + > int main(int argc, char *argv[]) > { > fd_set readfds, writefds; > @@ -307,6 +349,7 @@ int main(int argc, char *argv[]) > struct sigaction act; > GOptionContext *context; > GError *error = NULL; > + SpiceVDAgent *agent; > > context = g_option_context_new(NULL); > g_option_context_add_main_entries(context, entries, NULL); > @@ -357,20 +400,13 @@ reconnect: > execvp(argv[0], argv); > } > > - client = client_setup_sync(); > - if (client == NULL) { > - return 1; > + agent = g_object_new(SPICE_TYPE_VDAGENT, NULL); > + agent->parent_socket = parent_socket; > + if (!spice_vdagent_init_sync(agent)) { > + quit = 1; > + goto end_main; > } > > - x11 = vdagent_x11_create(client, debug, x11_sync); > - if (!x11) { > - udscs_destroy_connection(&client); > - return 1; > - } > - > - if (!vdagent_init_file_xfer()) > - syslog(LOG_WARNING, "File transfer is disabled"); > - > if (parent_socket) { > if (write(parent_socket, "OK", 2) != 2) > syslog(LOG_WARNING, "Parent already gone."); > @@ -378,12 +414,12 @@ reconnect: > parent_socket = 0; > } > > - while (client && !quit) { > + while (agent->conn && !quit) { > FD_ZERO(&readfds); > FD_ZERO(&writefds); > > - nfds = udscs_client_fill_fds(client, &readfds, &writefds); > - x11_fd = vdagent_x11_get_fd(x11); > + nfds = udscs_client_fill_fds(agent->conn, &readfds, &writefds); > + x11_fd = vdagent_x11_get_fd(agent->x11); > FD_SET(x11_fd, &readfds); > if (x11_fd >= nfds) > nfds = x11_fd + 1; > @@ -397,13 +433,12 @@ reconnect: > } > > if (FD_ISSET(x11_fd, &readfds)) > - vdagent_x11_do_read(x11); > - udscs_client_handle_fds(&client, &readfds, &writefds); > + vdagent_x11_do_read(agent->x11); > + udscs_client_handle_fds(&agent->conn, &readfds, &writefds); > } > > - vdagent_finalize_file_xfer(); > - vdagent_x11_destroy(x11, client == NULL); > - udscs_destroy_connection(&client); > +end_main: > + g_clear_object(&agent); > if (!quit && do_daemonize) > goto reconnect; > > diff --git a/src/vdagent/vdagent.h b/src/vdagent/vdagent.h > new file mode 100644 > index 0000000..f98c9b2 > --- /dev/null > +++ b/src/vdagent/vdagent.h > @@ -0,0 +1,54 @@ > +/* vdagent.h > + > + 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 <<a href="http://www.gnu.org/licenses/" rel="noreferrer" target="_blank">http://www.gnu.org/licenses/</a>>. > +*/ > + > +#ifndef __VDAGENT_H_ > +#define __VDAGENT_H_ > + > +#include <glib-object.h> > +#include "udscs.h" > +#include "x11.h" > +#include "file-xfers.h" > + > +G_BEGIN_DECLS > + > +#define SPICE_TYPE_VDAGENT (spice_vdagent_get_type ()) > +#define SPICE_VDAGENT(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), > SPICE_TYPE_VDAGENT, SpiceVDAgent)) > +#define SPICE_VDAGENT_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), > SPICE_TYPE_VDAGENT, SpiceVDAgentClass)) > +#define SPICE_IS_VDAGENT(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), > SPICE_TYPE_VDAGENT)) > +#define SPICE_IS_VDAGENT_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), > SPICE_TYPE_VDAGENT)) > +#define SPICE_VDAGENT_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), > SPICE_TYPE_VDAGENT, SpiceVDAgentClass)) > + > +typedef struct _SpiceVDAgent { Ids that starts with underscore and capital are not C99 complaint. </blockquote><div> </div><div>I'm not sure I understand that, why do we use these in spice-gtk then?</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + GObject parent; > + > + struct vdagent_x11 *x11; > + struct vdagent_file_xfers *xfers; > + struct udscs_connection *conn; > + > + int parent_socket; > +} SpiceVDAgent; > + > +typedef struct _SpiceVDAgentClass { ditto > + GObjectClass parent; > +} SpiceVDAgentClass; > + > +GType spice_vdagent_get_type(void); > + > +G_END_DECLS > + > +#endif > \ No newline at end of file Frediano </blockquote></div></div>