On Fri, Apr 15, 2016 at 03:01:37PM -0500, Jonathon Jongsma wrote:
> > @@ -394,21 +376,17 @@ static int
> > red_char_device_read_from_device(RedCharDevice *dev)
> >
> > static void red_char_device_client_send_queue_push(RedCharDeviceClient
> > *dev_client)
> > {
> > - RingItem *item;
> > - while ((item = ring_get_tail(&dev_client->send_queue)) &&
> > - red_char_device_can_send_to_client(dev_client)) {
> > - RedCharDeviceMsgToClientItem *msg_item;
> > -
> > - msg_item = SPICE_CONTAINEROF(item, RedCharDeviceMsgToClientItem,
> > link);
> > - ring_remove(item);
> > -
> > + while (!g_queue_is_empty(dev_client->send_queue)) {
> > + GList *msg;
>
> GList* ?? Doesn't this GQueue contain PipeItem*?
>
> > + if (!red_char_device_can_send_to_client(dev_client)) {
> > + break;
> > + }
> > + msg = g_queue_pop_tail(dev_client->send_queue);
> > + g_assert(msg != NULL);
> > dev_client->num_send_tokens--;
> > - red_char_device_send_msg_to_client(dev_client->dev,
> > - msg_item->msg,
> > + red_char_device_send_msg_to_client(dev_client->dev, msg->data,
>
> I think the second argument should simply be 'msg' instead of 'msg->data', no? I
> wonder why it works...
>
> > dev_client->client);
> > - pipe_item_unref(msg_item->msg);
> > - dev_client->send_queue_size--;
> > - free(msg_item);
> > + pipe_item_unref(msg);
>
>
> ...and here, we free this GList* variable by calling pipe_item_unref(). That's
> one of the pitfalls of making pipe_item_unref() accept a gpointer argument --
> the compiler didn't warn us...
Yes, looks like I got this hunk wrong. I must say that for most of these
changes, I did not go further than trying compilation, I did not
carefullly test them, so I'm not surprised to find such bugs :(
Might be better to delay merging some of these patches until I tested
them more carefully ;)
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel