[Spice-devel] [spice-server v1 1/2] mcc: early return and lower indentation

[Frediano Ziglio]

> 
> ---
>  server/main-channel-client.c | 97
>  +++++++++++++++++++++-----------------------
>  1 file changed, 47 insertions(+), 50 deletions(-)
> 
> diff --git a/server/main-channel-client.c b/server/main-channel-client.c
> index 12151a7..20cf932 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -376,59 +376,56 @@ void main_channel_client_handle_pong(MainChannelClient
> *mcc, SpiceMsgPing *ping,
>  
>      roundtrip = g_get_monotonic_time() - ping->timestamp;
>  
> -    if (ping->id == mcc->net_test_id) {
> -        switch (mcc->net_test_stage) {
> -            case NET_TEST_STAGE_WARMUP:
> -                mcc->net_test_id++;
> -                mcc->net_test_stage = NET_TEST_STAGE_LATENCY;
> -                mcc->latency = roundtrip;
> -                break;
> -            case NET_TEST_STAGE_LATENCY:
> -                mcc->net_test_id++;
> -                mcc->net_test_stage = NET_TEST_STAGE_RATE;
> -                mcc->latency = MIN(mcc->latency, roundtrip);
> -                break;
> -            case NET_TEST_STAGE_RATE:
> -                mcc->net_test_id = 0;
> -                if (roundtrip <= mcc->latency) {
> -                    // probably high load on client or server result with
> incorrect values
> -                    spice_printerr("net test: invalid values, latency %"
> PRIu64
> -                                   " roundtrip %" PRIu64 ". assuming high"
> -                                   "bandwidth", mcc->latency, roundtrip);
> -                    mcc->latency = 0;
> -                    mcc->net_test_stage = NET_TEST_STAGE_INVALID;
> -
> red_channel_client_start_connectivity_monitoring(&mcc->base,
> -
> CLIENT_CONNECTIVITY_TIMEOUT);
> -                    break;
> -                }
> -                mcc->bitrate_per_sec = (uint64_t)(NET_TEST_BYTES * 8) *
> 1000000
> -                    / (roundtrip - mcc->latency);
> -                mcc->net_test_stage = NET_TEST_STAGE_COMPLETE;
> -                spice_printerr("net test: latency %f ms, bitrate %"PRIu64"
> bps (%f Mbps)%s",
> -                               (double)mcc->latency / 1000,
> -                               mcc->bitrate_per_sec,
> -                               (double)mcc->bitrate_per_sec / 1024 / 1024,
> -                               main_channel_client_is_low_bandwidth(mcc) ? "
> LOW BANDWIDTH" : "");
> -                red_channel_client_start_connectivity_monitoring(&mcc->base,
> -
> CLIENT_CONNECTIVITY_TIMEOUT);
> -                break;
> -            default:
> -                spice_printerr("invalid net test stage, ping id %d test id
> %d stage %d",
> -                               ping->id,
> -                               mcc->net_test_id,
> -                               mcc->net_test_stage);
> -                mcc->net_test_stage = NET_TEST_STAGE_INVALID;
> -        }
> -        return;
> -    } else {
> -        /*
> -         * channel client monitors the connectivity using ping-pong messages
> -         */

Why did you remove this comment ?

> +    if (ping->id != mcc->net_test_id) {
>          red_channel_client_handle_message(rcc, size, SPICE_MSGC_PONG, ping);
> -    }
>  #ifdef RED_STATISTICS
> -    stat_update_value(red_channel_client_get_channel(rcc)->reds, roundtrip);
> +        stat_update_value(red_channel_client_get_channel(rcc)->reds,
> roundtrip);
>  #endif
> +        return;
> +    }
> +
> +    switch (mcc->net_test_stage) {
> +    case NET_TEST_STAGE_WARMUP:
> +        mcc->net_test_id++;
> +        mcc->net_test_stage = NET_TEST_STAGE_LATENCY;
> +        mcc->latency = roundtrip;
> +        break;
> +    case NET_TEST_STAGE_LATENCY:
> +        mcc->net_test_id++;
> +        mcc->net_test_stage = NET_TEST_STAGE_RATE;
> +        mcc->latency = MIN(mcc->latency, roundtrip);
> +        break;
> +    case NET_TEST_STAGE_RATE:
> +        mcc->net_test_id = 0;
> +        if (roundtrip <= mcc->latency) {
> +            // probably high load on client or server result with incorrect
> values
> +            spice_printerr("net test: invalid values, latency %" PRIu64
> +                           " roundtrip %" PRIu64 ". assuming high"
> +                           "bandwidth", mcc->latency, roundtrip);
> +            mcc->latency = 0;
> +            mcc->net_test_stage = NET_TEST_STAGE_INVALID;
> +            red_channel_client_start_connectivity_monitoring(&mcc->base,
> +
> CLIENT_CONNECTIVITY_TIMEOUT);
> +            break;
> +        }
> +        mcc->bitrate_per_sec = (uint64_t)(NET_TEST_BYTES * 8) * 1000000
> +            / (roundtrip - mcc->latency);
> +        mcc->net_test_stage = NET_TEST_STAGE_COMPLETE;
> +        spice_printerr("net test: latency %f ms, bitrate %"PRIu64" bps (%f
> Mbps)%s",
> +                       (double)mcc->latency / 1000,
> +                       mcc->bitrate_per_sec,
> +                       (double)mcc->bitrate_per_sec / 1024 / 1024,
> +                       main_channel_client_is_low_bandwidth(mcc) ? " LOW
> BANDWIDTH" : "");
> +        red_channel_client_start_connectivity_monitoring(&mcc->base,
> +
> CLIENT_CONNECTIVITY_TIMEOUT);
> +        break;
> +    default:
> +        spice_printerr("invalid net test stage, ping id %d test id %d stage
> %d",
> +                       ping->id,
> +                       mcc->net_test_id,
> +                       mcc->net_test_stage);
> +        mcc->net_test_stage = NET_TEST_STAGE_INVALID;
> +    }
>  }
>  
>  void main_channel_client_handle_migrate_end(MainChannelClient *mcc)

Otherwise it's fine.

Frediano