[Spice-devel] [PATCH vdagent 08/11] vdagent: Introduce SpiceVDAgent object
Frediano Ziglio
fziglio at redhat.com
Thu Sep 28 08:40:13 UTC 2017
> On Wed, Sep 27, 2017 at 1:02 PM Frediano Ziglio < fziglio at redhat.com > wrote:
> > >
>
> > > Related to ongoing work to use GMainLoop and GTK integration
>
> > >
>
> > > Signed-off-by: Victor Toso < victortoso at redhat.com >
>
> > > ---
>
> > > Makefile.am | 3 +
>
> > > configure.ac | 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/ configure.ac b/ configure.ac
>
> > > index d92b527..12b7d23 100644
>
> > > --- a/ configure.ac
>
> > > +++ b/ configure.ac
>
> > > @@ -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 ?
>
> Victor used "SessionAgent" in the earlier version, what about that?
I was suggesting something like
G_DEFINE_TYPE (VDAgent, vdagent, G_TYPE_OBJECT);
the "vdagent" (second argument) is the prefix used and there are already lot of functions
in this file with this prefix.
> > > -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?
>
> Sure.
> > > + if (G_OBJECT_CLASS(spice_vdagent_parent_class)->finalize)
>
> > this if is not necessary, see
> > https://developer.gnome.org/gobject/stable/howto-gobject-destruction.html
>
> OK.
> > > + 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 < http://www.gnu.org/licenses/ >.
>
> > > +*/
>
> > > +
>
> > > +#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.
>
> I'm not sure I understand that, why do we use these in spice-gtk then?
Historical reasons, C89 had not this. But after 18 years maybe we should consider to use C99 style.
> > > + 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
>
Frediano
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170928/da322360/attachment-0001.html>
More information about the Spice-devel
mailing list