[Spice-devel] [spice-server 3/6] Use enum rather than int in RedChannelClient{Latency, Connectivity}Monitor

[Jonathon Jongsma]

On Fri, 2017-04-07 at 16:31 +0200, Christophe Fergeau wrote:
> ---
>  server/red-channel-client.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/server/red-channel-client.c b/server/red-channel-
> client.c
> index c988170..eae5794 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -67,8 +67,15 @@ struct SpiceDataHeaderOpaque {
>      get_msg_size_proc get_msg_size;
>  };
>  
> +enum QosPingState {
> +    PING_STATE_NONE,
> +    PING_STATE_TIMER,
> +    PING_STATE_WARMUP,
> +    PING_STATE_LATENCY,
> +};
> +

Any particular reason that you didn't typedef this enum?

e.g.

typedef enum {
...
} QosPingState;

Looking through the rest of the code, I can't find any other
occurrences of something like the following:

enum enumName var_name;

So for consistency, I'd prefer to typedef the enum.


>  typedef struct RedChannelClientLatencyMonitor {
> -    int state;
> +    enum QosPingState state;
>      uint64_t last_pong_time;
>      SpiceTimer *timer;
>      uint32_t id;
> @@ -78,8 +85,15 @@ typedef struct RedChannelClientLatencyMonitor {
>      int64_t roundtrip;
>  } RedChannelClientLatencyMonitor;
>  
> +enum ConnectivityState {
> +    CONNECTIVITY_STATE_CONNECTED,
> +    CONNECTIVITY_STATE_BLOCKED,
> +    CONNECTIVITY_STATE_WAIT_PONG,
> +    CONNECTIVITY_STATE_DISCONNECTED,
> +};

again

> +
>  typedef struct RedChannelClientConnectivityMonitor {
> -    int state;
> +    enum ConnectivityState state;
>      bool sent_bytes;
>      bool received_bytes;
>      uint32_t timeout;
> @@ -197,20 +211,6 @@ enum {
>  #define PING_TEST_TIMEOUT_MS (MSEC_PER_SEC * 15)
>  #define PING_TEST_IDLE_NET_TIMEOUT_MS (MSEC_PER_SEC / 10)
>  
> -enum QosPingState {
> -    PING_STATE_NONE,
> -    PING_STATE_TIMER,
> -    PING_STATE_WARMUP,
> -    PING_STATE_LATENCY,
> -};
> -
> -enum ConnectivityState {
> -    CONNECTIVITY_STATE_CONNECTED,
> -    CONNECTIVITY_STATE_BLOCKED,
> -    CONNECTIVITY_STATE_WAIT_PONG,
> -    CONNECTIVITY_STATE_DISCONNECTED,
> -};
> -
>  typedef struct RedEmptyMsgPipeItem {
>      RedPipeItem base;
>      int msg;


Aside from my preference for typedefs mentioned above:

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>