[Spice-devel] [spice-server 1/3] Remove unneeded rcc->priv->latency_monitor.timer checks

[Christophe Fergeau]

On Tue, Apr 11, 2017 at 08:54:15AM -0400, Frediano Ziglio wrote:
> > 
> > red_channel_client_start_ping_timer(),
> > red_channel_client_cancel_ping_timer(),
> > red_channel_client_restart_ping_timer() are no-ops when
> > rcc->priv->latency_monitor.timer is NULL, so there is no need to
> > explicitly check it's not NULL before calling these functions.
> > 
> > Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> 
> Looking at code it seems the check is moved, not removed.

Yes and no, commit log is too terse (or commit is doing too much).
There are red_channel_client_{start,restart,cancel}_ping_timer
methods, start and cancel already had the check, and there were places
where the check was done before calling them.
restart indeed did not have the check, so I added it in this commit.

I rephrase the above, and add it to the log.

Christophe

> 
> Frediano
> 
> > ---
> >  server/red-channel-client.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> > 
> > diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> > index 802cd46..f9054ea 100644
> > --- a/server/red-channel-client.c
> > +++ b/server/red-channel-client.c
> > @@ -257,6 +257,9 @@ static void
> > red_channel_client_restart_ping_timer(RedChannelClient *rcc)
> >  {
> >      uint64_t passed, timeout;
> >  
> > +    if (!rcc->priv->latency_monitor.timer) {
> > +        return;
> > +    }
> >      passed = (spice_get_monotonic_time_ns() -
> >      rcc->priv->latency_monitor.last_pong_time) / NSEC_PER_MILLISEC;
> >      timeout = PING_TEST_IDLE_NET_TIMEOUT_MS;
> >      if (passed  < PING_TEST_TIMEOUT_MS) {
> > @@ -649,8 +652,7 @@ static void red_channel_client_msg_sent(RedChannelClient
> > *rcc)
> >          spice_assert(rcc->priv->send_data.header.data != NULL);
> >          red_channel_client_begin_send_message(rcc);
> >      } else {
> > -        if (rcc->priv->latency_monitor.timer
> > -            && !red_channel_client_is_blocked(rcc)
> > +        if (!red_channel_client_is_blocked(rcc)
> >              && g_queue_is_empty(&rcc->priv->pipe)) {
> >              /* It is possible that the socket will become idle, so we may be
> >              able to test latency */
> >              red_channel_client_restart_ping_timer(rcc);
> > @@ -969,9 +971,7 @@ static void
> > red_channel_client_seamless_migration_done(RedChannelClient *rcc)
> >      rcc->priv->wait_migrate_data = FALSE;
> >  
> >      if (red_client_seamless_migration_done_for_channel(rcc->priv->client)) {
> > -        if (rcc->priv->latency_monitor.timer) {
> > -            red_channel_client_start_ping_timer(rcc,
> > PING_TEST_IDLE_NET_TIMEOUT_MS);
> > -        }
> > +        red_channel_client_start_ping_timer(rcc,
> > PING_TEST_IDLE_NET_TIMEOUT_MS);
> >          if (rcc->priv->connectivity_monitor.timer) {
> >              SpiceCoreInterfaceInternal *core =
> >              red_channel_get_core_interface(rcc->priv->channel);
> >              core->timer_start(core, rcc->priv->connectivity_monitor.timer,
> > @@ -982,9 +982,7 @@ static void
> > red_channel_client_seamless_migration_done(RedChannelClient *rcc)
> >  
> >  void red_channel_client_semi_seamless_migration_complete(RedChannelClient
> >  *rcc)
> >  {
> > -    if (rcc->priv->latency_monitor.timer) {
> > -        red_channel_client_start_ping_timer(rcc,
> > PING_TEST_IDLE_NET_TIMEOUT_MS);
> > -    }
> > +    red_channel_client_start_ping_timer(rcc, PING_TEST_IDLE_NET_TIMEOUT_MS);
> >  }
> >  
> >  bool red_channel_client_is_waiting_for_migrate_data(RedChannelClient *rcc)
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170411/b6c6ea85/attachment.sig>