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

[Pavel Grunt]

On Wed, 2016-09-14 at 05:53 -0400, Frediano Ziglio wrote:
> > 
> > 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.

Not sure what do you mean?

> 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.
It is called because agent should not send this message to the client,
i would consider it as an error.

Pavel
> 
> Frediano
>