On Tue, Sep 13, 2016 at 11:29:17PM +0200, Pavel Grunt wrote:
> Reuse and handle the return value from agent_msg_filter_process_data
Not overly convinced code is better this way
(agent_msg_filter_process_data is simpler, the 2 callers are more
complicated ;)
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> ---
> v2 per Frediano's comments:
> - clarified the commit message
> - documented return values
> - added fall through comment
> ---
> server/reds.c | 69 +++++++++++++++++++++++++++--------------------------------
> 1 file changed, 31 insertions(+), 38 deletions(-)
>
> diff --git a/server/reds.c b/server/reds.c
> index 5ff50cd..d0adcaa 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -763,37 +763,23 @@ 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 the #AgentMsgFilterResult value:
> + AGENT_MSG_FILTER_OK if the buffer can be forwarded,
> + AGENT_MSG_FILTER_DISCARD if the buffer can be discarded
> + other values on error
> +*/
> +static AgentMsgFilterResult vdi_port_read_buf_process(RedCharDeviceVDIPort *dev,
> + RedVDIReadBuf *buf)
> {
> - AgentMsgFilterResult 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_MONITORS_CONFIG:
> - g_warn_if_reached();
> - /* fall through */
> - 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;
> }
> }
>
> @@ -884,7 +870,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;
> @@ -903,12 +888,16 @@ 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);
> - }
> + case AGENT_MSG_FILTER_MONITORS_CONFIG:
> + g_warn_if_reached();
> + /* fall through */
> + case AGENT_MSG_FILTER_PROTO_ERROR:
> + reds_agent_remove(reds);
> + /* fall through */
> + case AGENT_MSG_FILTER_DISCARD:
> red_pipe_item_unref(&dispatch_buf->base);
> }
> }
> @@ -1278,22 +1267,26 @@ 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;
> + case AGENT_MSG_FILTER_MONITORS_CONFIG:
> + g_warn_if_reached();
> + /* fall through */
> + case AGENT_MSG_FILTER_PROTO_ERROR:
> + reds_agent_remove(reds);
> + /* fall through */
> + case AGENT_MSG_FILTER_DISCARD:
> red_pipe_item_unref(&read_buf->base);
> }
>
> --
> 2.10.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel