On Thu, 2016-09-15 at 11:16 -0400, Frediano Ziglio wrote:
> >
> >
> > More Ring cleanup. At the moment, we only support a single client,
> > so
> > this is only a one-element list
> > ---
> > Changes in v2:
> > - simplified red_char_device_finalize() to avoid using
> > g_list_last()
> >
> > server/char-device.c | 68
> > ++++++++++++++++++++--------------------------------
> > 1 file changed, 26 insertions(+), 42 deletions(-)
> >
> > diff --git a/server/char-device.c b/server/char-device.c
> > index f826524..1b99aa9 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -32,7 +32,6 @@
> >
> > typedef struct RedCharDeviceClient RedCharDeviceClient;
> > struct RedCharDeviceClient {
> > - RingItem link;
> > RedCharDevice *dev;
> > RedClient *client;
> > int do_flow_control;
> > @@ -58,8 +57,7 @@ struct RedCharDevicePrivate {
> > SpiceTimer *write_to_dev_timer;
> > uint64_t num_self_tokens;
> >
> > - Ring clients; /* list of RedCharDeviceClient */
> > - uint32_t num_clients;
> > + GList *clients; /* list of RedCharDeviceClient */
> >
> > uint64_t client_tokens_interval; /* frequency of returning
> > tokens to the
> > client */
> > SpiceCharDeviceInstance *sin;
> > @@ -207,8 +205,7 @@ static void
> > red_char_device_client_free(RedCharDevice
> > *dev,
> > dev->priv->cur_write_buf->client = NULL;
> > }
> >
> > - dev->priv->num_clients--;
> > - ring_remove(&dev_client->link);
> > + dev->priv->clients = g_list_remove(dev->priv->clients,
> > dev_client);
> > free(dev_client);
> > }
> >
> > @@ -222,12 +219,11 @@ static void
> > red_char_device_handle_client_overflow(RedCharDeviceClient
> > *dev_clie
> > static RedCharDeviceClient
> > *red_char_device_client_find(RedCharDevice *dev,
> > RedClient
> > *client)
> > {
> > - RingItem *item;
> > + GList *item;
> >
> > - RING_FOREACH(item, &dev->priv->clients) {
> > - RedCharDeviceClient *dev_client;
> > + for (item = dev->priv->clients; item != NULL; item = item-
> > >next) {
> > + RedCharDeviceClient *dev_client = item->data;
> >
> > - dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient,
> > link);
> > if (dev_client->client == client) {
> > return dev_client;
> > }
> > @@ -253,13 +249,11 @@ static int
> > red_char_device_can_send_to_client(RedCharDeviceClient *dev_client)
> >
> > static uint64_t red_char_device_max_send_tokens(RedCharDevice
> > *dev)
> > {
> > - RingItem *item;
> > + GList *item;
> > uint64_t max = 0;
> >
> > - RING_FOREACH(item, &dev->priv->clients) {
> > - RedCharDeviceClient *dev_client;
> > -
> > - dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient,
> > link);
> > + for (item = dev->priv->clients; item != NULL; item = item-
> > >next) {
> > + RedCharDeviceClient *dev_client = item->data;
> >
> > if (!dev_client->do_flow_control) {
> > max = ~0;
> > @@ -295,12 +289,11 @@ static void
> > red_char_device_add_msg_to_client_queue(RedCharDeviceClient
> > *dev_cli
> > static void red_char_device_send_msg_to_clients(RedCharDevice
> > *dev,
> > RedPipeItem *msg)
> > {
> > - RingItem *item, *next;
> > + GList *l;
> >
> > - RING_FOREACH_SAFE(item, next, &dev->priv->clients) {
> > - RedCharDeviceClient *dev_client;
> > + for (l = dev->priv->clients; l != NULL; l = l->next) {
> > + RedCharDeviceClient *dev_client = l->data;
> >
> > - dev_client = SPICE_CONTAINEROF(item, RedCharDeviceClient,
> > link);
> > if (red_char_device_can_send_to_client(dev_client)) {
> > dev_client->num_send_tokens--;
> > spice_assert(g_queue_is_empty(dev_client-
> > >send_queue));
>
> The _SAFE loop here was necessary as there are combinations were
> the client is removed from the list during the loop.
> This will make l vanish before l->next is read.
> Even if there is only a client this can happen.
Hmm, I couldn't find a code path where this happened. But there is a
comment within the loop suggesting that it can happen, so it's better
to use the safer loop style. Sorry for changing it without enough
diligence.
jonathon
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel