On Mon, Sep 11, 2017 at 11:15:46AM +0100, Frediano Ziglio wrote: > Instead of inserting the marker after the item (the tail of the queue > is the first item to send) and then have to wait again if for > the specific item place the marker before the item so waiting for > the marker to be sent assure that we sent also the item. > This avoids having to call red_channel_client_wait_outgoing_item > and possibly the case where the item was not queued and > red_channel_client_wait_outgoing_item returning TRUE even if > the item was not sent as required. As usual, I'd rework a bit the commit log (more punctuation/new lines, more accurate references to typenames/variable names, ..): "Currently, red_channel_client_wait_pipe_item_sent() inserts a MarkerItem which will sent before the item we want to wait for: the tail of the queue is the first item to send, and the function uses red_channel_client_pipe_add_before_pos(). Then, if the marker has been successfully sent, the function calls red_channel_client_wait_outgoing_item to wait for 'item' to be sent. Instead of doing this, we can add the MarkerItem to the queue so that it's sent after 'item' (ie, insert it _before_ 'item' in the queue). This way, when the marker is marked as having been sent, we'll also know that 'item' has been sent. This avoids having to call red_channel_client_wait_outgoing_item and possibly the case where the item was not queued and red_channel_client_wait_outgoing_item returning TRUE even if the item was not sent as required." I believe the way this function works now is a result of the last hunk of https://cgit.freedesktop.org/spice/spice/commit/?id=c59b2884a2f7fc953fdb263085830b65e8bdcaef + the introduction of MarkerItem (I guess when switching to using a GQueue). Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> Christophe > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > --- > server/red-channel-client.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/server/red-channel-client.c b/server/red-channel-client.c > index f0a25ecfd..d9333ba6f 100644 > --- a/server/red-channel-client.c > +++ b/server/red-channel-client.c > @@ -1569,6 +1569,19 @@ void red_channel_client_pipe_add_after_pos(RedChannelClient *rcc, > g_queue_insert_after(&rcc->priv->pipe, pipe_item_pos, item); > } > > +static void > +red_channel_client_pipe_add_before_pos(RedChannelClient *rcc, > + RedPipeItem *item, > + GList *pipe_item_pos) > +{ > + spice_assert(pipe_item_pos); > + if (!prepare_pipe_add(rcc, item)) { > + return; > + } > + > + g_queue_insert_before(&rcc->priv->pipe, pipe_item_pos, item); > +} > + > void red_channel_client_pipe_add_after(RedChannelClient *rcc, > RedPipeItem *item, > RedPipeItem *pos) > @@ -1779,7 +1792,7 @@ bool red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc, > red_pipe_item_init(&mark_item->base, RED_PIPE_ITEM_TYPE_MARKER); > mark_item->item_in_pipe = true; > red_pipe_item_ref(&mark_item->base); > - red_channel_client_pipe_add_after_pos(rcc, &mark_item->base, item_pos); > + red_channel_client_pipe_add_before_pos(rcc, &mark_item->base, item_pos); > > for (;;) { > red_channel_client_receive(rcc); > @@ -1799,10 +1812,8 @@ bool red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc, > // still on the queue > spice_warning("timeout"); > return FALSE; > - } else { > - return red_channel_client_wait_outgoing_item(rcc, > - timeout == -1 ? -1 : end_time - spice_get_monotonic_time_ns()); > } > + return TRUE; > } > > bool red_channel_client_wait_outgoing_item(RedChannelClient *rcc, > -- > 2.13.5 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel