[Spice-devel] [PATCH spice v2 1/2] agent-filter: Use enum as return value

[Frediano Ziglio]

> 
> Also remove unused AGENT_MSG_FILTER_END
> ---
> v2 per Christophe's comments:
>  - removed AGENT_MSG_FILTER_END
>  - avoid 'default' and explicitly check for the enum value
> ---
>  server/agent-msg-filter.c |  4 ++--
>  server/agent-msg-filter.h | 11 +++++------
>  server/reds.c             |  7 +++++--
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
> index a11f624..7921fe7 100644
> --- a/server/agent-msg-filter.c
> +++ b/server/agent-msg-filter.c
> @@ -48,8 +48,8 @@ void agent_msg_filter_init(AgentMsgFilter *filter,
>      filter->discard_all = discard_all;
>  }
>  
> -int agent_msg_filter_process_data(AgentMsgFilter *filter,
> -                                  const uint8_t *data, uint32_t len)
> +AgentMsgFilterResult agent_msg_filter_process_data(AgentMsgFilter *filter,
> +                                                   const uint8_t *data,
> uint32_t len)
>  {
>      struct VDAgentMessage msg_header;
>  
> diff --git a/server/agent-msg-filter.h b/server/agent-msg-filter.h
> index d61f8d4..b4d8e72 100644
> --- a/server/agent-msg-filter.h
> +++ b/server/agent-msg-filter.h
> @@ -25,17 +25,16 @@
>  #include <glib.h>
>  
>  /* Possible return values for agent_msg_filter_process_data */
> -enum {
> +typedef enum {
>      AGENT_MSG_FILTER_OK,
>      AGENT_MSG_FILTER_DISCARD,
>      AGENT_MSG_FILTER_PROTO_ERROR,
>      AGENT_MSG_FILTER_MONITORS_CONFIG,
> -    AGENT_MSG_FILTER_END
> -};
> +} AgentMsgFilterResult;
>  
>  typedef struct AgentMsgFilter {
>      int msg_data_to_read;
> -    int result;
> +    AgentMsgFilterResult result;
>      gboolean copy_paste_enabled;
>      gboolean file_xfer_enabled;
>      gboolean use_client_monitors_config;
> @@ -49,7 +48,7 @@ void agent_msg_filter_init(AgentMsgFilter *filter,
>  void agent_msg_filter_config(AgentMsgFilter *filter,
>                               gboolean copy_paste, gboolean file_xfer,
>                               gboolean use_client_monitors_config);
> -int agent_msg_filter_process_data(AgentMsgFilter *filter,
> -                                  const uint8_t *data, uint32_t len);
> +AgentMsgFilterResult agent_msg_filter_process_data(AgentMsgFilter *filter,
> +                                                   const uint8_t *data,
> uint32_t len);
>  
>  #endif
> diff --git a/server/reds.c b/server/reds.c
> index 800107b..5ff50cd 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -767,7 +767,7 @@ static void vdi_port_read_buf_release(uint8_t *data, void
> *opaque)
>  static gboolean vdi_port_read_buf_process(RedCharDeviceVDIPort *dev,
>                                            RedVDIReadBuf *buf, gboolean
>                                            *error)
>  {
> -    int res;
> +    AgentMsgFilterResult res;
>  
>      *error = FALSE;
>  
> @@ -780,6 +780,9 @@ static gboolean
> vdi_port_read_buf_process(RedCharDeviceVDIPort *dev,
>              return TRUE;
>          case AGENT_MSG_FILTER_DISCARD:
>              return FALSE;
> +        case AGENT_MSG_FILTER_MONITORS_CONFIG:
> +            g_warn_if_reached();
> +            /* fall through */
>          case AGENT_MSG_FILTER_PROTO_ERROR:
>              *error = TRUE;
>              return FALSE;
> @@ -1199,7 +1202,7 @@ void reds_on_main_agent_data(RedsState *reds,
> MainChannelClient *mcc, void *mess
>  {
>      RedCharDeviceVDIPort *dev = reds->agent_dev;
>      VDIChunkHeader *header;
> -    int res;
> +    AgentMsgFilterResult res;
>  
>      res =
>      agent_msg_filter_process_data(&reds->agent_dev->priv->write_filter,
>                                          message, size);

This patch changes quite a big the behavior.
AGENT_MSG_FILTER_MONITORS_CONFIG behaved more like AGENT_MSG_FILTER_DISCARD
than AGENT_MSG_FILTER_PROTO_ERROR and I don't understand why g_warn_if_reached
is now called.

Frediano