I think the rationale of this is to remove the error return parameter and
make vdi_port_read_buf_process easier/shorter.
>
> ---
> server/reds.c | 55 ++++++++++++++++++++-----------------------------------
> 1 file changed, 20 insertions(+), 35 deletions(-)
>
> diff --git a/server/reds.c b/server/reds.c
> index 800107b..a13a14a 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -763,34 +763,18 @@ static void vdi_port_read_buf_release(uint8_t *data,
> void *opaque)
> red_pipe_item_unref((RedPipeItem *)opaque);
> }
>
> -/* returns TRUE if the buffer can be forwarded */
> -static gboolean vdi_port_read_buf_process(RedCharDeviceVDIPort *dev,
> - RedVDIReadBuf *buf, gboolean
> *error)
> +/* returns AGENT_MSG_FILTER_OK if the buffer can be forwarded,
> + AGENT_MSG_FILTER_PROTO_ERROR on error */
Why AGENT_MSG_FILTER_DISCARD is not documented ?
> +static int vdi_port_read_buf_process(RedCharDeviceVDIPort *dev,
> RedVDIReadBuf *buf)
Don't we have an enum for this return instead of using int ?
enum {
AGENT_MSG_FILTER_OK,
AGENT_MSG_FILTER_DISCARD,
AGENT_MSG_FILTER_PROTO_ERROR,
AGENT_MSG_FILTER_MONITORS_CONFIG,
AGENT_MSG_FILTER_END
};
mumble... weird enum. Beside AGENT_MSG_FILTER_END which is
not used there is OK but also a AGENT_MSG_FILTER_MONITORS_CONFIG
which I suppose it's OK too... but this is not related to this
patch.
> {
> - int res;
> -
> - *error = FALSE;
> -
> switch (dev->priv->vdi_chunk_header.port) {
> - case VDP_CLIENT_PORT: {
> - res = agent_msg_filter_process_data(&dev->priv->read_filter,
> - buf->data, buf->len);
> - switch (res) {
> - case AGENT_MSG_FILTER_OK:
> - return TRUE;
> - case AGENT_MSG_FILTER_DISCARD:
> - return FALSE;
> - case AGENT_MSG_FILTER_PROTO_ERROR:
> - *error = TRUE;
> - return FALSE;
> - }
> - }
> + case VDP_CLIENT_PORT:
> + return agent_msg_filter_process_data(&dev->priv->read_filter,
> buf->data, buf->len);
> case VDP_SERVER_PORT:
> - return FALSE;
> + return AGENT_MSG_FILTER_DISCARD;
> default:
> spice_warning("invalid port");
> - *error = TRUE;
> - return FALSE;
> + return AGENT_MSG_FILTER_PROTO_ERROR;
> }
> }
>
> @@ -881,7 +865,6 @@ static RedPipeItem
> *vdi_port_read_one_msg_from_device(SpiceCharDeviceInstance *s
> dev->priv->read_state = VDI_PORT_READ_STATE_READ_DATA;
> }
> case VDI_PORT_READ_STATE_READ_DATA: {
> - gboolean error = FALSE;
> n = sif->read(reds->vdagent, dev->priv->receive_pos,
> dev->priv->receive_len);
> if (!n) {
> return NULL;
> @@ -900,12 +883,13 @@ static RedPipeItem
> *vdi_port_read_one_msg_from_device(SpiceCharDeviceInstance *s
> } else {
> dev->priv->read_state = VDI_PORT_READ_STATE_GET_BUFF;
> }
> - if (vdi_port_read_buf_process(reds->agent_dev, dispatch_buf,
> &error)) {
> + switch (vdi_port_read_buf_process(reds->agent_dev,
> dispatch_buf)) {
> + case AGENT_MSG_FILTER_OK:
> return &dispatch_buf->base;
> - } else {
> - if (error) {
> - reds_agent_remove(reds);
> - }
> + default:
> + case AGENT_MSG_FILTER_PROTO_ERROR:
> + reds_agent_remove(reds);
I would add a fall through comment here, at least to make some
automatic tools more happy.
> + case AGENT_MSG_FILTER_DISCARD:
> red_pipe_item_unref(&dispatch_buf->base);
> }
> }
> @@ -1275,22 +1259,23 @@ void reds_on_main_channel_migrate(RedsState *reds,
> MainChannelClient *mcc)
> if (agent_dev->priv->read_filter.msg_data_to_read ||
> read_data_len > sizeof(VDAgentMessage)) { /* msg header has been
> read */
> RedVDIReadBuf *read_buf = agent_dev->priv->current_read_buf;
> - gboolean error = FALSE;
>
> spice_debug("push partial read %u (msg first chunk? %d)",
> read_data_len,
> !agent_dev->priv->read_filter.msg_data_to_read);
>
> read_buf->len = read_data_len;
> - if (vdi_port_read_buf_process(reds->agent_dev, read_buf, &error)) {
> + switch (vdi_port_read_buf_process(reds->agent_dev, read_buf)) {
> + case AGENT_MSG_FILTER_OK:
> 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);
> - }
> + break;
> + default:
> + case AGENT_MSG_FILTER_PROTO_ERROR:
> + reds_agent_remove(reds);
> + case AGENT_MSG_FILTER_DISCARD:
> red_pipe_item_unref(&read_buf->base);
> }
>
Looks like copy&paste but this is not a regression, caller code of
vdi_port_read_buf_process were very similar even before.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel