[Spice-devel] [PATCH 05/15] Remove dependency of vdi_port_read_buf_process on RedsState

[Frediano Ziglio]

> 
> From: Christophe Fergeau <cfergeau at redhat.com>
> 
> This makes it easier to move the VDIPort API to a different file, and
> make it as self-contained as possible.
> ---
>  server/reds.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index 736dca6..08913e8 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -632,14 +632,16 @@ static void vdi_port_read_buf_release(uint8_t *data,
> void *opaque)
>  }
>  
>  /* returns TRUE if the buffer can be forwarded */
> -static int vdi_port_read_buf_process(RedsState *reds, VDIReadBuf *buf)
> +static gboolean vdi_port_read_buf_process(VDIPortState *state, VDIReadBuf
> *buf, gboolean *error)
>  {
> -    VDIPortState *state = &reds->agent_state;
>      int res;
>  
> +    *error = FALSE;
> +
>      switch (state->vdi_chunk_header.port) {
>      case VDP_CLIENT_PORT: {
> -        res = agent_msg_filter_process_data(&state->read_filter, reds,
> +        res = agent_msg_filter_process_data(&state->read_filter,
> +
> spice_char_device_get_server(state->base),

Actually you can get RedsState (and you are using it) from state->base.
So why this complication of having to pass back a flag to say that
we need to call reds_agent_remove instead of just replacing reds
argument with state but continue to call reds_agent_remove here?

>                                              buf->data, buf->len);
>          switch (res) {
>          case AGENT_MSG_FILTER_OK:
> @@ -647,7 +649,7 @@ static int vdi_port_read_buf_process(RedsState *reds,
> VDIReadBuf *buf)
>          case AGENT_MSG_FILTER_DISCARD:
>              return FALSE;
>          case AGENT_MSG_FILTER_PROTO_ERROR:
> -            reds_agent_remove(reds);
> +            *error = TRUE;
>              return FALSE;
>          }
>      }
> @@ -655,7 +657,7 @@ static int vdi_port_read_buf_process(RedsState *reds,
> VDIReadBuf *buf)
>          return FALSE;
>      default:
>          spice_warning("invalid port");
> -        reds_agent_remove(reds);
> +        *error = TRUE;
>          return FALSE;
>      }
>  }
> @@ -739,7 +741,8 @@ static SpiceCharDeviceMsgToClient
> *vdi_port_read_one_msg_from_device(SpiceCharDe
>              state->message_receive_len -= state->receive_len;
>              state->read_state = VDI_PORT_READ_STATE_READ_DATA;
>          }
> -        case VDI_PORT_READ_STATE_READ_DATA:
> +        case VDI_PORT_READ_STATE_READ_DATA: {
> +            gboolean error = FALSE;
>              n = sif->read(reds->vdagent, state->receive_pos,
>              state->receive_len);
>              if (!n) {
>                  return NULL;
> @@ -758,11 +761,15 @@ static SpiceCharDeviceMsgToClient
> *vdi_port_read_one_msg_from_device(SpiceCharDe
>              } else {
>                  state->read_state = VDI_PORT_READ_STATE_GET_BUFF;
>              }
> -            if (vdi_port_read_buf_process(reds, dispatch_buf)) {
> +            if (vdi_port_read_buf_process(&reds->agent_state, dispatch_buf,
> &error)) {
>                  return dispatch_buf;
>              } else {
> +                if (error) {
> +                    reds_agent_remove(reds);
> +                }
>                  vdi_port_read_buf_unref(dispatch_buf);
>              }
> +        }
>          } /* END switch */
>      } /* END while */
>      return NULL;
> @@ -1122,18 +1129,22 @@ void reds_on_main_channel_migrate(RedsState *reds,
> MainChannelClient *mcc)
>      if (agent_state->read_filter.msg_data_to_read ||
>          read_data_len > sizeof(VDAgentMessage)) { /* msg header has been
>          read */
>          VDIReadBuf *read_buf = agent_state->current_read_buf;
> +        gboolean error = FALSE;
>  
>          spice_debug("push partial read %u (msg first chunk? %d)",
>          read_data_len,
>                      !agent_state->read_filter.msg_data_to_read);
>  
>          read_buf->len = read_data_len;
> -        if (vdi_port_read_buf_process(reds, read_buf)) {
> +        if (vdi_port_read_buf_process(&reds->agent_state, read_buf, &error))
> {
>              main_channel_client_push_agent_data(mcc,
>                                                  read_buf->data,
>                                                  read_buf->len,
>                                                  vdi_port_read_buf_release,
>                                                  read_buf);
>          } else {
> +            if (error) {
> +               reds_agent_remove(reds);
> +            }
>              vdi_port_read_buf_unref(read_buf);
>          }
>  

Frediano