[Spice-devel] [PATCH linux/vd-agent 08/11] clipboard: gobject-ify VDAgentClipboards
Jakub Janku
jjanku at redhat.com
Mon Mar 25 10:46:34 UTC 2019
Hi,
On Mon, Mar 25, 2019, 11:34 AM Marc-André Lureau <marcandre.lureau at gmail.com>
wrote:
> Hi
>
> On Mon, Mar 25, 2019 at 10:26 AM Frediano Ziglio <fziglio at redhat.com>
> wrote:
> >
> > >
> > > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > >
> > > This will allow easier lifecycle management,
> > > and usage of gtk_clipboard_set_with_owner()
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > ---
> > > src/vdagent/clipboard.c | 67 +++++++++++++++++++++++++++--------------
> > > src/vdagent/clipboard.h | 12 +++++---
> > > src/vdagent/vdagent.c | 7 +++--
> > > 3 files changed, 56 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/src/vdagent/clipboard.c b/src/vdagent/clipboard.c
> > > index 1e49248..cf6e344 100644
> > > --- a/src/vdagent/clipboard.c
> > > +++ b/src/vdagent/clipboard.c
> > > @@ -76,15 +76,25 @@ typedef struct {
> > > } Selection;
> > > #endif
> > >
> > > -struct VDAgentClipboards {
> > > -#ifdef WITH_GTK
> > > +struct _VDAgentClipboards {
> >
> > Can we use C99 complaints identifiers?
>
> I didn't think much about the struct identifiers.
>
> fwiw, glib/gobject/gtk uses "struct _Foo" everywhere.
>
I think it's necessary as long as you want to use the new GObject macro.
G_DECLARE_FINAL_TYPE typedefs VdagentClipboards as struct _VDAgentClipboards
afaik.
Cheers,
Jakub
>
> >
> > > + GObject parent;
> > > +
> > > struct udscs_connection *conn;
> > > - Selection selections[SELECTION_COUNT];
> > > +
> > > +#ifdef WITH_GTK
> > > + Selection selections[SELECTION_COUNT];
> > > #else
> > > struct vdagent_x11 *x11;
> > > #endif
> > > };
> > >
> > > +struct _VDAgentClipboardsClass
> > > +{
> >
> > 2 structure style declaration in one patch
>
> Hmm? are you talking about braces indentation here?
>
> >
> > > + GObjectClass parent;
> > > +};
> > > +
> > > +G_DEFINE_TYPE(VDAgentClipboards, vdagent_clipboards, G_TYPE_OBJECT)
> > > +
> > > #ifdef WITH_GTK
> > > static const struct {
> > > guint type;
> > > @@ -453,43 +463,56 @@ err:
> > > #endif
> > > }
> > >
> > > -VDAgentClipboards *vdagent_clipboards_init(struct vdagent_x11
> *x11,
> > > - struct udscs_connection
> *conn)
> > > +static void
> > > +vdagent_clipboards_init(VDAgentClipboards *self)
> > > {
> > > -#ifdef WITH_GTK
> > > - guint sel_id;
> > > -#endif
> > > +}
> > > +
> > > +VDAgentClipboards *vdagent_clipboards_new(struct vdagent_x11 *x11)
> > > +{
> > > + VDAgentClipboards *self = g_object_new(VDAGENT_TYPE_CLIPBOARDS,
> NULL);
> > >
> > > - VDAgentClipboards *c;
> > > - c = g_new0(VDAgentClipboards, 1);
> > > #ifndef WITH_GTK
> > > - c->x11 = x11;
> > > + self->x11 = x11;
> > > #else
> > > - c->conn = conn;
> > > + guint sel_id;
> > >
> > > for (sel_id = 0; sel_id < SELECTION_COUNT; sel_id++) {
> > > GtkClipboard *clipboard = gtk_clipboard_get(sel_atom[sel_id]);
> > > - c->selections[sel_id].clipboard = clipboard;
> > > + self->selections[sel_id].clipboard = clipboard;
> > > g_signal_connect(G_OBJECT(clipboard), "owner-change",
> > > - G_CALLBACK(clipboard_owner_change_cb), c);
> > > + G_CALLBACK(clipboard_owner_change_cb), self);
> > > }
> > > #endif
> > >
> > > - return c;
> > > + return self;
> > > }
> > >
> > > -void vdagent_clipboards_finalize(VDAgentClipboards *c, gboolean
> conn_alive)
> > > +void
> > > +vdagent_clipboards_set_conn(VDAgentClipboards *self, struct
> udscs_connection
> > > *conn)
> > > +{
> > > + self->conn = conn;
> > > +}
> > > +
> > > +static void vdagent_clipboards_dispose(GObject *obj)
> > > {
> > > #ifdef WITH_GTK
> > > + VDAgentClipboards *self = VDAGENT_CLIPBOARDS(obj);
> > > guint sel_id;
> > > +
> > > for (sel_id = 0; sel_id < SELECTION_COUNT; sel_id++)
> > > -
> > > g_signal_handlers_disconnect_by_func(c->selections[sel_id].clipboard,
> > > - G_CALLBACK(clipboard_owner_change_cb), c);
> > > +
> > >
> g_signal_handlers_disconnect_by_func(self->selections[sel_id].clipboard,
> > > + G_CALLBACK(clipboard_owner_change_cb), self);
> > >
> > > - if (conn_alive == FALSE)
> > > - c->conn = NULL;
> > > - vdagent_clipboards_release_all(c);
> > > + if (self->conn)
> > > + vdagent_clipboards_release_all(self);
> > > #endif
> > > +}
> > > +
> > > +static void
> > > +vdagent_clipboards_class_init(VDAgentClipboardsClass *klass)
> > > +{
> > > + GObjectClass *oclass = G_OBJECT_CLASS(klass);
> > >
> > > - g_free(c);
> > > + oclass->dispose = vdagent_clipboards_dispose;
> > > }
> > > diff --git a/src/vdagent/clipboard.h b/src/vdagent/clipboard.h
> > > index f819b49..cd8eacb 100644
> > > --- a/src/vdagent/clipboard.h
> > > +++ b/src/vdagent/clipboard.h
> > > @@ -19,16 +19,18 @@
> > > #ifndef __VDAGENT_CLIPBOARD_H
> > > #define __VDAGENT_CLIPBOARD_H
> > >
> > > -#include <glib.h>
> > > +#include <glib-object.h>
> > >
> > > #include "x11.h"
> > > #include "udscs.h"
> > >
> > > -typedef struct VDAgentClipboards VDAgentClipboards;
> > > +#define VDAGENT_TYPE_CLIPBOARDS vdagent_clipboards_get_type()
> > > +G_DECLARE_FINAL_TYPE(VDAgentClipboards, vdagent_clipboards, VDAGENT,
> > > CLIPBOARDS, GObject)
> > >
> > > -VDAgentClipboards *vdagent_clipboards_init(struct vdagent_x11
> *x11,
> > > - struct udscs_connection
> *conn);
> > > -void vdagent_clipboards_finalize(VDAgentClipboards *c, gboolean
> conn_alive);
> > > +VDAgentClipboards *vdagent_clipboards_new(struct vdagent_x11 *x11);
> > > +
> > > +void vdagent_clipboards_set_conn(VDAgentClipboards *self,
> > > + struct udscs_connection *conn);
> > >
> > > void vdagent_clipboard_request(VDAgentClipboards *c, guint sel_id,
> guint
> > > type);
> > >
> > > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> > > index 61aeac7..bfc0d5f 100644
> > > --- a/src/vdagent/vdagent.c
> > > +++ b/src/vdagent/vdagent.c
> > > @@ -165,8 +165,8 @@ static void vdagent_quit_loop(VDAgent *agent)
> > > {
> > > /* other GMainLoop(s) might be running, quit them before
> agent->loop */
> > > if (agent->clipboards) {
> > > - vdagent_clipboards_finalize(agent->clipboards, agent->conn !=
> NULL);
> > > - agent->clipboards = NULL;
> > > + vdagent_clipboards_set_conn(agent->clipboards, agent->conn);
> > > + g_clear_object(&agent->clipboards);
> > > }
> > > if (agent->loop)
> > > g_main_loop_quit(agent->loop);
> > > @@ -400,7 +400,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_clipboards_init(agent->x11,
> agent->conn);
> > > + agent->clipboards = vdagent_clipboards_new(agent->x11);
> > > + vdagent_clipboards_set_conn(agent->clipboards, agent->conn);
> >
> > Why not using a constructor property to pass agent->conn?
>
> Because it's not needed at construct time, however we need to change
> the value when the conn goes away.
>
> >
> > >
> > > if (parent_socket != -1) {
> > > if (write(parent_socket, "OK", 2) != 2)
> >
> > Beside style I didn't have a look at other stuff
>
> thanks
>
>
>
> --
> Marc-André Lureau
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190325/b5ac51a1/attachment.html>
More information about the Spice-devel
mailing list