> > 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_STATIC_STRINGS > | > G_PARAM_READWRITE)); > - g_object_class_install_property(object_class, > - PROP_OPAQUE, > - g_param_spec_pointer("opaque", > - "opaque", > - "User data to pass > to callbacks", > - G_PARAM_STATIC_STRINGS > | > - G_PARAM_READWRITE)); > - > } > > 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, > + SpiceCharDeviceInstance *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(client), > 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, > + SpiceCharDeviceInstance > *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(RedCharDeviceSmartcard > > 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_READWRITE | > + > 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel