[Spice-devel] [PATCH v2 2/3] char-device: add 'self' param to vfuncs
Jonathon Jongsma
jjongsma at redhat.com
Thu Nov 3 17:10:20 UTC 2016
On Thu, 2016-11-03 at 11:02 -0400, Frediano Ziglio wrote:
> >
> >
> > Add a 'self' parameter to all of the char device virtual functions
> > so
> > that we don't have to play games with the 'opaque' pointer.
> > ---
> > Changes in v2:
> > - remove 'opaque' property from red_char_device_spicevmc_new()
> > - use g_param_spec_object() instead of g_param_spec_pointer() for
> > 'channel'
> > property
> >
> > server/char-device.c | 31 ++-----------
> > server/char-device.h | 20 ++++----
> > server/reds.c | 24 +++++-----
> > server/smartcard-channel-client.c | 2 +-
> > server/smartcard.c | 32 ++++++-------
> > server/spicevmc.c | 97
> > ++++++++++++++++++++++++++++++++-------
> > 6 files changed, 125 insertions(+), 81 deletions(-)
> >
> > diff --git a/server/char-device.c b/server/char-device.c
> > index 318bf3c..3b70066 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -66,7 +66,6 @@ struct RedCharDevicePrivate {
> > int during_read_from_device;
> > int during_write_to_device;
> >
> > - void *opaque;
> > SpiceServer *reds;
> > };
> >
> > @@ -88,7 +87,6 @@ enum {
> > PROP_SPICE_SERVER,
> > PROP_CLIENT_TOKENS_INTERVAL,
> > PROP_SELF_TOKENS,
> > - PROP_OPAQUE
> > };
> >
> > static void
> > red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer
> > *write_buf);
> > @@ -99,7 +97,7 @@
> > red_char_device_read_one_msg_from_device(RedCharDevice
> > *dev)
> > {
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >
> > - return klass->read_one_msg_from_device(dev->priv->sin,
> > dev->priv->opaque);
> > + return klass->read_one_msg_from_device(dev, dev->priv->sin);
> > }
> >
> > static void
> > @@ -109,7 +107,7 @@
> > red_char_device_send_msg_to_client(RedCharDevice *dev,
> > {
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >
> > - klass->send_msg_to_client(msg, client, dev->priv->opaque);
> > + klass->send_msg_to_client(dev, msg, client);
> > }
> >
> > static void
> > @@ -119,7 +117,7 @@
> > red_char_device_send_tokens_to_client(RedCharDevice *dev,
> > {
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >
> > - klass->send_tokens_to_client(client, tokens, dev->priv-
> > >opaque);
> > + klass->send_tokens_to_client(dev, client, tokens);
> > }
> >
> > static void
> > @@ -128,7 +126,7 @@
> > red_char_device_on_free_self_token(RedCharDevice *dev)
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >
> > if (klass->on_free_self_token != NULL) {
> > - klass->on_free_self_token(dev->priv->opaque);
> > + klass->on_free_self_token(dev);
> > }
> > }
> >
> > @@ -137,7 +135,7 @@ red_char_device_remove_client(RedCharDevice
> > *dev,
> > RedClient *client)
> > {
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >
> > - klass->remove_client(client, dev->priv->opaque);
> > + klass->remove_client(dev, client);
> > }
> >
> > static void
> > red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> > @@ -686,11 +684,6 @@ void
> > red_char_device_reset_dev_instance(RedCharDevice
> > *dev,
> > g_object_notify(G_OBJECT(dev), "sin");
> > }
> >
> > -void *red_char_device_opaque_get(RedCharDevice *dev)
> > -{
> > - return dev->priv->opaque;
> > -}
> > -
> > void red_char_device_destroy(RedCharDevice *char_dev)
> > {
> > g_return_if_fail(RED_IS_CHAR_DEVICE(char_dev));
> > @@ -1041,9 +1034,6 @@
> > red_char_device_get_property(GObject *object,
> > case PROP_SELF_TOKENS:
> > g_value_set_uint64(value, self->priv-
> > >num_self_tokens);
> > break;
> > - case PROP_OPAQUE:
> > - g_value_set_pointer(value, self->priv->opaque);
> > - break;
> > default:
> > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> > pspec);
> > }
> > @@ -1071,9 +1061,6 @@
> > red_char_device_set_property(GObject *object,
> > case PROP_SELF_TOKENS:
> > self->priv->num_self_tokens =
> > g_value_get_uint64(value);
> > break;
> > - case PROP_OPAQUE:
> > - self->priv->opaque = g_value_get_pointer(value);
> > - break;
> > default:
> > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> > pspec);
> > }
> > @@ -1156,14 +1143,6 @@
> > red_char_device_class_init(RedCharDeviceClass *klass)
> > 0,
> > G_MAXUINT64, 0,
> > G_PARAM_ST
> > ATIC_STRINGS
> > |
> > G_PARAM_RE
> > ADWRITE));
> > - g_object_class_install_property(object_class,
> > - PROP_OPAQUE,
> > - g_param_spec_pointer("opaque",
> > - "opaque",
> > - "User
> > data to pass
> > to callbacks",
> > - G_PARAM_STAT
> > IC_STRINGS
> > >
> > >
> > - G_PARAM_READ
> > WRITE));
> > -
> > }
> >
> > static void
> > diff --git a/server/char-device.h b/server/char-device.h
> > index 44e9504..3b87023 100644
> > --- a/server/char-device.h
> > +++ b/server/char-device.h
> > @@ -56,25 +56,27 @@ struct RedCharDeviceClass
> >
> > /* reads from the device till reaching a msg that should be
> > sent to the
> > client,
> > * or till the reading fails */
> > - RedPipeItem*
> > (*read_one_msg_from_device)(SpiceCharDeviceInstance *sin,
> > - void *opaque);
> > + RedPipeItem* (*read_one_msg_from_device)(RedCharDevice *self,
> > + SpiceCharDeviceInstan
> > ce *sin);
> > /* after this call, the message is unreferenced */
> > - void (*send_msg_to_client)(RedPipeItem *msg,
> > - RedClient *client,
> > - void *opaque);
> > + void (*send_msg_to_client)(RedCharDevice *self,
> > + RedPipeItem *msg,
> > + RedClient *client);
> >
> > /* The cb is called when a predefined number of write buffers
> > were
> > consumed by the
> > * device */
> > - void (*send_tokens_to_client)(RedClient *client, uint32_t
> > tokens, void
> > *opaque);
> > + void (*send_tokens_to_client)(RedCharDevice *self,
> > + RedClient *client,
> > + uint32_t tokens);
> >
> > /* The cb is called when a server (self) message that was
> > addressed to
> > the device,
> > * has been completely written to it */
> > - void (*on_free_self_token)(void *opaque);
> > + void (*on_free_self_token)(RedCharDevice *self);
> >
> > /* This cb is called if it is recommended to remove the client
> > * due to slow flow or due to some other error.
> > * The called instance should disconnect the client, or at
> > least the
> > corresponding channel */
> > - void (*remove_client)(RedClient *client, void *opaque);
> > + void (*remove_client)(RedCharDevice *self, RedClient *client);
> > };
> >
> > GType red_char_device_get_type(void) G_GNUC_CONST;
> > @@ -159,8 +161,6 @@ void
> > red_char_device_reset_dev_instance(RedCharDevice
> > *dev,
> > SpiceCharDeviceInstance
> > *sin);
> > void red_char_device_destroy(RedCharDevice *dev);
> >
> > -void *red_char_device_opaque_get(RedCharDevice *dev);
> > -
> > /* only one client is supported */
> > void red_char_device_migrate_data_marshall(RedCharDevice *dev,
> > SpiceMarshaller *m);
> > diff --git a/server/reds.c b/server/reds.c
> > index 30b9165..5daf9bd 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -817,11 +817,11 @@ static void
> > vdi_port_read_buf_free(RedPipeItem *base)
> >
> > /* reads from the device till completes reading a message that is
> > addressed
> > to the client,
> > * or otherwise, when reading from the device fails */
> > -static RedPipeItem
> > *vdi_port_read_one_msg_from_device(SpiceCharDeviceInstance *sin,
> > - void
> > *opaque)
> > +static RedPipeItem
> > *vdi_port_read_one_msg_from_device(RedCharDevice *self,
> > +
> > SpiceCharDeviceInstance
> > *sin)
> > {
> > RedsState *reds;
> > - RedCharDeviceVDIPort *dev = RED_CHAR_DEVICE_VDIPORT(sin->st);
> > + RedCharDeviceVDIPort *dev = RED_CHAR_DEVICE_VDIPORT(self);
> > SpiceCharDeviceInterface *sif;
> > RedVDIReadBuf *dispatch_buf;
> > int n;
> > @@ -894,9 +894,9 @@ static RedPipeItem
> > *vdi_port_read_one_msg_from_device(SpiceCharDeviceInstance *s
> > }
> >
> > /* after calling this, we unref the message, and the ref is in the
> > instance
> > side */
> > -static void vdi_port_send_msg_to_client(RedPipeItem *msg,
> > - RedClient *client,
> > - void *opaque)
> > +static void vdi_port_send_msg_to_client(RedCharDevice *self,
> > + RedPipeItem *msg,
> > + RedClient *client)
> > {
> > RedVDIReadBuf *agent_data_buf = (RedVDIReadBuf *)msg;
> >
> > @@ -908,15 +908,17 @@ static void
> > vdi_port_send_msg_to_client(RedPipeItem
> > *msg,
> > agent_data_buf);
> > }
> >
> > -static void vdi_port_send_tokens_to_client(RedClient *client,
> > uint32_t
> > tokens, void *opaque)
> > +static void vdi_port_send_tokens_to_client(RedCharDevice *self,
> > + RedClient *client,
> > + uint32_t tokens)
> > {
> > main_channel_client_push_agent_tokens(red_client_get_main(clie
> > nt),
> > tokens);
> > }
> >
> > -static void vdi_port_on_free_self_token(void *opaque)
> > +static void vdi_port_on_free_self_token(RedCharDevice *self)
> > {
> > - RedsState *reds = opaque;
> > + RedsState *reds = red_char_device_get_server(self);
> >
> > if (reds->inputs_channel && reds->pending_mouse_event) {
> > spice_debug("pending mouse event");
> > @@ -924,7 +926,8 @@ static void vdi_port_on_free_self_token(void
> > *opaque)
> > }
> > }
> >
> > -static void vdi_port_remove_client(RedClient *client, void
> > *opaque)
> > +static void vdi_port_remove_client(RedCharDevice *self,
> > + RedClient *client)
> > {
> > red_channel_client_shutdown(RED_CHANNEL_CLIENT(red_client_get_
> > main(client)));
> > }
> > @@ -4514,6 +4517,5 @@ static RedCharDeviceVDIPort
> > *red_char_device_vdi_port_new(RedsState *reds)
> > "spice-server", reds,
> > "client-tokens-interval",
> > (guint64)REDS_TOKENS_TO_SEND,
> > "self-tokens",
> > (guint64)REDS_NUM_INTERNAL_AGENT_MESSAGES,
> > - "opaque", reds,
> > NULL);
> > }
> > diff --git a/server/smartcard-channel-client.c
> > b/server/smartcard-channel-client.c
> > index 30b2249..d00a6b2 100644
> > --- a/server/smartcard-channel-client.c
> > +++ b/server/smartcard-channel-client.c
> > @@ -273,7 +273,7 @@ static void
> > smartcard_channel_client_remove_reader(SmartCardChannelClient *scc,
> > return;
> > }
> >
> > - dev = red_char_device_opaque_get(char_device->st);
> > + dev = RED_CHAR_DEVICE_SMARTCARD(char_device->st);
> > spice_assert(scc->priv->smartcard == dev);
> > if (!smartcard_char_device_notify_reader_remove(dev)) {
> > smartcard_channel_client_push_error(RED_CHANNEL_CLIENT(scc
> > ),
> > diff --git a/server/smartcard.c b/server/smartcard.c
> > index 53919c0..5b18abe 100644
> > --- a/server/smartcard.c
> > +++ b/server/smartcard.c
> > @@ -149,10 +149,10 @@ static void
> > smartcard_read_buf_prepare(RedCharDeviceSmartcard *dev,
> > VSCMsgHeader
> > }
> > }
> >
> > -static RedPipeItem
> > *smartcard_read_msg_from_device(SpiceCharDeviceInstance
> > *sin,
> > - void *opaque)
> > +static RedPipeItem *smartcard_read_msg_from_device(RedCharDevice
> > *self,
> > + SpiceCharDevice
> > Instance
> > *sin)
> > {
> > - RedCharDeviceSmartcard *dev = opaque;
> > + RedCharDeviceSmartcard *dev = RED_CHAR_DEVICE_SMARTCARD(self);
> > SpiceCharDeviceInterface *sif =
> > spice_char_device_get_interface(sin);
> > VSCMsgHeader *vheader = (VSCMsgHeader*)dev->priv->buf;
> > int n;
> > @@ -186,11 +186,11 @@ static RedPipeItem
> > *smartcard_read_msg_from_device(SpiceCharDeviceInstance *sin,
> > return NULL;
> > }
> >
> > -static void smartcard_send_msg_to_client(RedPipeItem *msg,
> > - RedClient *client,
> > - void *opaque)
> > +static void smartcard_send_msg_to_client(RedCharDevice *self,
> > + RedPipeItem *msg,
> > + RedClient *client)
> > {
> > - RedCharDeviceSmartcard *dev = opaque;
> > + RedCharDeviceSmartcard *dev = RED_CHAR_DEVICE_SMARTCARD(self);
> > RedChannelClient *rcc = RED_CHANNEL_CLIENT(dev->priv->scc);
> >
> > spice_assert(dev->priv->scc &&
> > @@ -199,14 +199,16 @@ static void
> > smartcard_send_msg_to_client(RedPipeItem
> > *msg,
> > smartcard_channel_client_pipe_add_push(rcc, msg);
> > }
> >
> > -static void smartcard_send_tokens_to_client(RedClient *client,
> > uint32_t
> > tokens, void *opaque)
> > +static void smartcard_send_tokens_to_client(RedCharDevice *self,
> > + RedClient *client,
> > + uint32_t tokens)
> > {
> > spice_error("not implemented");
> > }
> >
> > -static void smartcard_remove_client(RedClient *client, void
> > *opaque)
> > +static void smartcard_remove_client(RedCharDevice *self, RedClient
> > *client)
> > {
> > - RedCharDeviceSmartcard *dev = opaque;
> > + RedCharDeviceSmartcard *dev = RED_CHAR_DEVICE_SMARTCARD(self);
> > RedChannelClient *rcc = RED_CHANNEL_CLIENT(dev->priv->scc);
> >
> > spice_printerr("smartcard dev %p, client %p", dev, client);
> > @@ -247,7 +249,7 @@ RedMsgItem
> > *smartcard_char_device_on_message_from_device(RedCharDeviceSmartcar
> > d
> >
> > static int smartcard_char_device_add_to_readers(RedsState *reds,
> > SpiceCharDeviceInstance *char_device)
> > {
> > - RedCharDeviceSmartcard *dev =
> > red_char_device_opaque_get(char_device->st);
> > + RedCharDeviceSmartcard *dev =
> > RED_CHAR_DEVICE_SMARTCARD(char_device->st);
> >
> > if (g_smartcard_readers.num >= SMARTCARD_MAX_READERS) {
> > return -1;
> > @@ -272,7 +274,7 @@ SpiceCharDeviceInstance
> > *smartcard_readers_get_unattached(void)
> > RedCharDeviceSmartcard* dev;
> >
> > for (i = 0; i < g_smartcard_readers.num; ++i) {
> > - dev =
> > red_char_device_opaque_get(g_smartcard_readers.sin[i]->st);
> > + dev =
> > RED_CHAR_DEVICE_SMARTCARD(g_smartcard_readers.sin[i]->st);
> > if (!dev->priv->scc) {
> > return g_smartcard_readers.sin[i];
> > }
> > @@ -291,8 +293,6 @@ static RedCharDeviceSmartcard
> > *smartcard_device_new(RedsState *reds, SpiceCharDe
> > "self-tokens", ~0ULL,
> > NULL);
> >
> > - g_object_set(char_dev, "opaque", char_dev, NULL);
> > -
> > return RED_CHAR_DEVICE_SMARTCARD(char_dev);
> > }
> >
> > @@ -336,7 +336,7 @@ void
> > smartcard_char_device_notify_reader_add(RedCharDeviceSmartcard
> > *dev)
> > void smartcard_char_device_attach_client(SpiceCharDeviceInstance
> > *char_device,
> > SmartCardChannelClient
> > *scc)
> > {
> > - RedCharDeviceSmartcard *dev =
> > red_char_device_opaque_get(char_device->st);
> > + RedCharDeviceSmartcard *dev =
> > RED_CHAR_DEVICE_SMARTCARD(char_device->st);
> > int client_added;
> >
> > spice_assert(!smartcard_channel_client_get_char_device(scc) &&
> > !dev->priv->scc);
> > @@ -499,7 +499,7 @@ void
> > smartcard_channel_write_to_reader(RedCharDeviceWriteBuffer
> > *write_buf)
> >
> > spice_assert(vheader->reader_id <= g_smartcard_readers.num);
> > sin = g_smartcard_readers.sin[vheader->reader_id];
> > - dev = (RedCharDeviceSmartcard
> > *)red_char_device_opaque_get(sin->st);
> > + dev = RED_CHAR_DEVICE_SMARTCARD(sin->st);
> > spice_assert(!dev->priv->scc ||
> > dev ==
> > smartcard_channel_client_get_device(dev->priv-
> > >scc));
> > /* protocol requires messages to be in network endianess */
> > diff --git a/server/spicevmc.c b/server/spicevmc.c
> > index 8e23248..4f99a24 100644
> > --- a/server/spicevmc.c
> > +++ b/server/spicevmc.c
> > @@ -86,10 +86,13 @@ struct RedCharDeviceSpiceVmcClass
> > RedCharDeviceClass parent_class;
> > };
> >
> > +typedef struct RedVmcChannel RedVmcChannel;
> > +typedef struct RedVmcChannelClass RedVmcChannelClass;
> > +
> > static GType red_char_device_spicevmc_get_type(void) G_GNUC_CONST;
> > static RedCharDevice
> > *red_char_device_spicevmc_new(SpiceCharDeviceInstance
> > *sin,
> > RedsState
> > *reds,
> > - void *opaque);
> > + RedVmcChannel
> > *vmc_channel);
> >
> > G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc,
> > RED_TYPE_CHAR_DEVICE)
> >
> > @@ -349,10 +352,11 @@ static RedVmcPipeItem*
> > try_compress_lz4(RedVmcChannel
> > *state, int n, RedVmcPipeI
> > }
> > #endif
> >
> > -static RedPipeItem
> > *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *sin,
> > - void
> > *opaque)
> > +static RedPipeItem
> > *spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
> > +
> > SpiceCharDeviceInstance
> > *sin)
> > {
> > - RedVmcChannel *state = opaque;
> > + RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
> > + RedVmcChannel *state = RED_VMC_CHANNEL(vmc->channel);
> > SpiceCharDeviceInterface *sif;
> > RedVmcPipeItem *msg_item;
> > int n;
> > @@ -394,11 +398,12 @@ static RedPipeItem
> > *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *
> > }
> > }
> >
> > -static void spicevmc_chardev_send_msg_to_client(RedPipeItem *msg,
> > - RedClient *client,
> > - void *opaque)
> > +static void spicevmc_chardev_send_msg_to_client(RedCharDevice
> > *self,
> > + RedPipeItem *msg,
> > + RedClient *client)
> > {
> > - RedVmcChannel *state = opaque;
> > + RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
> > + RedVmcChannel *state = RED_VMC_CHANNEL(vmc->channel);
> >
> > spice_assert(red_channel_client_get_client(state->rcc) ==
> > client);
> > red_pipe_item_ref(msg);
> > @@ -432,16 +437,18 @@ static void
> > spicevmc_port_send_event(RedChannelClient
> > *rcc, uint8_t event)
> > red_channel_client_pipe_add_push(rcc, &item->base);
> > }
> >
> > -static void spicevmc_char_dev_send_tokens_to_client(RedClient
> > *client,
> > - uint32_t
> > tokens,
> > - void *opaque)
> > +static void spicevmc_char_dev_send_tokens_to_client(RedCharDevice
> > *self,
> > + RedClient
> > *client,
> > + uint32_t
> > tokens)
> > {
> > spice_printerr("Not implemented!");
> > }
> >
> > -static void spicevmc_char_dev_remove_client(RedClient *client,
> > void *opaque)
> > +static void spicevmc_char_dev_remove_client(RedCharDevice *self,
> > + RedClient *client)
> > {
> > - RedVmcChannel *state = opaque;
> > + RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
> > + RedVmcChannel *state = RED_VMC_CHANNEL(vmc->channel);
> >
> > spice_printerr("vmc state %p, client %p", state, client);
> > spice_assert(state->rcc &&
> > @@ -767,6 +774,7 @@ red_vmc_channel_class_init(RedVmcChannelClass
> > *klass)
> > channel_class->handle_migrate_flush_mark =
> > spicevmc_channel_client_handle_migrate_flush_mark;
> > channel_class->handle_migrate_data =
> > spicevmc_channel_client_handle_migrate_data;
> >
> > +
> > g_object_class_install_property(object_class,
> > PROP_DEVICE_INSTANCE,
> > g_param_spec_pointer("device-
> > instance",
>
> Extra space change.
>
> >
> > @@ -885,14 +893,15 @@ void spicevmc_device_disconnect(RedsState
> > *reds,
> > SpiceCharDeviceInstance *sin)
> > SPICE_GNUC_VISIBLE void
> > spice_server_port_event(SpiceCharDeviceInstance
> > *sin, uint8_t event)
> > {
> > RedVmcChannel *state;
> > + RedCharDeviceSpiceVmc *device = RED_CHAR_DEVICE_SPICEVMC(sin-
> > >st);
> >
> > if (sin->st == NULL) {
> > spice_warning("no SpiceCharDeviceState attached to
> > instance %p",
> > sin);
> > return;
> > }
> >
> > - /* FIXME */
> > - state = (RedVmcChannel
> > *)red_char_device_opaque_get((RedCharDevice*)sin->st);
> > + state = RED_VMC_CHANNEL(device->channel);
> > +
> > if (event == SPICE_PORT_EVENT_OPENED) {
> > state->port_opened = TRUE;
> > } else if (event == SPICE_PORT_EVENT_CLOSED) {
> > @@ -914,6 +923,48 @@ red_char_device_spicevmc_dispose(GObject
> > *object)
> > g_clear_object(&self->channel);
> > }
> >
> > +enum {
> > + PROP_CHAR_DEVICE_0,
> > + PROP_CHAR_DEVICE_CHANNEL,
> > +};
> > +
> > +static void
> > +red_char_device_spicevmc_get_property(GObject *object,
> > + guint property_id,
> > + GValue *value,
> > + GParamSpec *pspec)
> > +{
> > + RedCharDeviceSpiceVmc *self =
> > RED_CHAR_DEVICE_SPICEVMC(object);
> > +
> > + switch (property_id)
> > + {
> > + case PROP_CHAR_DEVICE_CHANNEL:
> > + g_value_set_object(value, self->channel);
> > + break;
> > + default:
> > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> > pspec);
> > + }
> > +}
> > +
> > +static void
> > +red_char_device_spicevmc_set_property(GObject *object,
> > + guint property_id,
> > + const GValue *value,
> > + GParamSpec *pspec)
> > +{
> > + RedCharDeviceSpiceVmc *self =
> > RED_CHAR_DEVICE_SPICEVMC(object);
> > +
> > + switch (property_id)
> > + {
> > + case PROP_CHAR_DEVICE_CHANNEL:
> > + g_clear_object(&self->channel);
> > + self->channel = g_value_dup_object(value);
> > + break;
> > + default:
> > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> > pspec);
> > + }
> > +}
> > +
> > static void
> > red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass
> > *klass)
> > {
> > @@ -921,11 +972,23 @@
> > red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass
> > *klass)
> > RedCharDeviceClass *char_dev_class =
> > RED_CHAR_DEVICE_CLASS(klass);
> >
> > object_class->dispose = red_char_device_spicevmc_dispose;
> > + object_class->get_property =
> > red_char_device_spicevmc_get_property;
> > + object_class->set_property =
> > red_char_device_spicevmc_set_property;
> >
> > char_dev_class->read_one_msg_from_device =
> > spicevmc_chardev_read_msg_from_dev;
> > char_dev_class->send_msg_to_client =
> > spicevmc_chardev_send_msg_to_client;
> > char_dev_class->send_tokens_to_client =
> > spicevmc_char_dev_send_tokens_to_client;
> > char_dev_class->remove_client =
> > spicevmc_char_dev_remove_client;
> > +
> > + g_object_class_install_property(object_class,
> > + PROP_CHAR_DEVICE_CHANNEL,
> > + g_param_spec_object("channel",
> > + "channel",
> > + "Channel
> > associated
> > with the char device",
> > +
> > RED_TYPE_VMC_CHANNEL,
> > + G_PARAM_RE
> > ADWRITE |
> > +
> > G_PARAM_CONSTRUCT_ONLY
> > >
> > >
> > +
> > G_PARAM_STATIC_STRINGS));
> > }
> >
> > static void
>
> This fixes the object/pointer critical error but not the leak.
> On 1/3 you created a strong reference from device to channel with
> this you are creating a strong reference from channel to device so
> a circular reference.
> Before these patches both device and channel had 1 as reference
> counter
> now device has 2 reference, channel 1.
> I used this https://cgit.freedesktop.org/~fziglio/spice-server/commit
> /?h=check&id=c0b6b3e2ccbbcb0a83c798477e0d8aafd1507d1d
> to check. Not in a shape to be applied surely.
>
> >
> > @@ -936,13 +999,13 @@
> > red_char_device_spicevmc_init(RedCharDeviceSpiceVmc
> > *self)
> > static RedCharDevice *
> > red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
> > RedsState *reds,
> > - void *opaque)
> > + RedVmcChannel *vmc_channel)
> > {
> > return g_object_new(RED_TYPE_CHAR_DEVICE_SPICEVMC,
> > "sin", sin,
> > "spice-server", reds,
> > "client-tokens-interval", 0ULL,
> > "self-tokens", ~0ULL,
> > - "opaque", opaque,
> > + "channel", vmc_channel,
> > NULL);
> > }
>
> I think should be reds -->> device -->> channel, channel -> device (-
> ->> strong, -> weak).
>
> Frediano
I think that makes sense. Because the channel can't reasonably live
without a device, but the device could theoretically exist without a
channel. Another option perhaps is for the server to hold a strong
reference to both the devices and the channels.
reds-->> device -> channel
reds-->> channel -> device
Maybe your intial suggestion is better though.
Ownership of all of these objects a little bit unclear right now since
up to this point we've just let them expire when the process exits. I'm
still thinking about the implications of all of the ownership
scenarios.
Jonathon
More information about the Spice-devel
mailing list