> > 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 I think here should be red_channel_client_pipe_add_after_pos. > 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." > Other part is perfect. > 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). > Not sure... looks like the patch you are referring is doing something else. > Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> > > Christophe > Frediano > > 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