[Spice-devel] [spice-server v2 2/3] channel: Remove red_channel_client_disconnect_if_pending_send()

[Frediano Ziglio]

> 
> Wasn’t there a discussion about also renaming the function? I believe
> “pending_send” is a symptom, not the reason for disconnecting.
> 
> Christophe
> 

The discussion was on red_channel_wait_all_sent, we decide a comment
was enough. Said that we can have always updates.

Are you talking about red_channel_client_disconnect_if_pending_send ?

Frediano


> > On 12 Sep 2017, at 16:15, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > 
> > There is exactly one user in RedChannel, and this can be reimplemented
> > using already public RedChannelClient API. No need for an extra
> > function very specialized function with a not great name.
> > 
> > This commit thus removes one method from RedChannelClient public API,
> > and replaces it with an equivalent private helper in RedChannel.
> > 
> > Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> > ---
> > Changes since v1
> > - use FOREACH_CLIENT
> > - slightly better commit log
> > 
> > server/red-channel-client.c |  9 ---------
> > server/red-channel-client.h |  1 -
> > server/red-channel.c        | 15 ++++++++++++++-
> > 3 files changed, 14 insertions(+), 11 deletions(-)
> > 
> > diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> > index 34202c492..8f7308628 100644
> > --- a/server/red-channel-client.c
> > +++ b/server/red-channel-client.c
> > @@ -1848,15 +1848,6 @@ bool
> > red_channel_client_wait_outgoing_item(RedChannelClient *rcc,
> >     }
> > }
> > 
> > -void red_channel_client_disconnect_if_pending_send(RedChannelClient *rcc)
> > -{
> > -    if (red_channel_client_is_blocked(rcc) ||
> > !g_queue_is_empty(&rcc->priv->pipe)) {
> > -        red_channel_client_disconnect(rcc);
> > -    } else {
> > -        spice_assert(red_channel_client_no_item_being_sent(rcc));
> > -    }
> > -}
> > -
> > gboolean red_channel_client_no_item_being_sent(RedChannelClient *rcc)
> > {
> >     return !rcc || (rcc->priv->send_data.size == 0);
> > diff --git a/server/red-channel-client.h b/server/red-channel-client.h
> > index f5e04df78..732fbdd59 100644
> > --- a/server/red-channel-client.h
> > +++ b/server/red-channel-client.h
> > @@ -143,7 +143,6 @@ bool
> > red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,
> >                                             int64_t timeout);
> > bool red_channel_client_wait_outgoing_item(RedChannelClient *rcc,
> >                                            int64_t timeout);
> > -void red_channel_client_disconnect_if_pending_send(RedChannelClient *rcc);
> > 
> > RedChannel* red_channel_client_get_channel(RedChannelClient *rcc);
> > 
> > diff --git a/server/red-channel.c b/server/red-channel.c
> > index c24b97722..b8f4f54e8 100644
> > --- a/server/red-channel.c
> > +++ b/server/red-channel.c
> > @@ -627,6 +627,19 @@ uint32_t red_channel_sum_pipes_size(RedChannel
> > *channel)
> >     return sum;
> > }
> > 
> > +static void red_channel_disconnect_if_pending_send(RedChannel *channel)
> > +{
> > +    RedChannelClient *rcc;
> > +
> > +    FOREACH_CLIENT(channel, rcc) {
> > +        if (red_channel_client_is_blocked(rcc) ||
> > !red_channel_client_pipe_is_empty(rcc)) {
> > +            red_channel_client_disconnect(rcc);
> > +        } else {
> > +            spice_assert(red_channel_client_no_item_being_sent(rcc));
> > +        }
> > +    }
> > +}
> > +
> > bool red_channel_wait_all_sent(RedChannel *channel,
> >                                int64_t timeout)
> > {
> > @@ -654,7 +667,7 @@ bool red_channel_wait_all_sent(RedChannel *channel,
> >     if (max_pipe_size || blocked) {
> >         spice_warning("timeout: pending out messages exist (pipe-size %u,
> >         blocked %d)",
> >                       max_pipe_size, blocked);
> > -        red_channel_apply_clients(channel,
> > red_channel_client_disconnect_if_pending_send);
> > +        red_channel_disconnect_if_pending_send(channel);
> >         return FALSE;
> >     } else {
> >         spice_assert(red_channel_no_item_being_sent(channel));