[Spice-devel] [PATCH vdagent-linux 4/6] vdagentd: move code to do_agent_xorg_resolution()

[Victor Toso]

Hi,

On Tue, Sep 04, 2018 at 06:40:49PM +0200, Jakub Janků wrote:
> Move code handling VDAGENTD_GUEST_XORG_RESOLUTION messages
> from agent_read_complete() to a separate function to make
> the switch statement less cluttered.
> 
> Remove unnecessary res variable.

You could also use something like resolution_size to compute
sizeof(struct vdagentd_guest_xorg_resolution) to keep code
shorter and save one sizeof operation (one of the uses of res
variable).

> 
> Signed-off-by: Jakub Janků <jjanku at redhat.com>
> ---
>  src/vdagentd/vdagentd.c | 62 +++++++++++++++++++++--------------------
>  1 file changed, 32 insertions(+), 30 deletions(-)
> 
> diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> index 1ac30ee..a1962be 100644
> --- a/src/vdagentd/vdagentd.c
> +++ b/src/vdagentd/vdagentd.c
> @@ -861,43 +861,45 @@ static void agent_disconnect(struct udscs_connection *conn)
>      g_free(agent_data);
>  }
>  
> -static void agent_read_complete(struct udscs_connection **connp,
> -    struct udscs_message_header *header, uint8_t *data)
> +static void do_agent_xorg_resolution(struct udscs_connection    **connp,
> +                                     struct udscs_message_header *header,
> +                                     guint8                      *data)
>  {
>      struct agent_data *agent_data = udscs_get_user_data(*connp);
> +    guint n = header->size / sizeof(struct vdagentd_guest_xorg_resolution);
> +
> +    /* Detect older version session agent, but don't disconnect, as
> +     * that stops it from getting the VDAGENTD_VERSION message, and then
> +     * it will never re-exec the new version... */
> +    if (header->arg1 == 0 && header->arg2 == 0) {
> +        syslog(LOG_INFO, "got old session agent xorg resolution message, "
> +                         "ignoring");
> +        return;
> +    }
>  
> -    switch (header->type) {
> -    case VDAGENTD_GUEST_XORG_RESOLUTION: {
> -        struct vdagentd_guest_xorg_resolution *res;
> -        int n = header->size / sizeof(*res);
> -
> -        /* Detect older version session agent, but don't disconnect, as
> -           that stops it from getting the VDAGENTD_VERSION message, and then
> -           it will never re-exec the new version... */
> -        if (header->arg1 == 0 && header->arg2 == 0) {
> -            syslog(LOG_INFO, "got old session agent xorg resolution message, "
> -                             "ignoring");
> -            return;
> -        }
> +    if (header->size != n * sizeof(struct vdagentd_guest_xorg_resolution)) {
> +        syslog(LOG_ERR, "guest xorg resolution message has wrong size, "
> +                        "disconnecting agent");
> +        udscs_destroy_connection(connp);
> +        return;
> +    }
>  
> -        if (header->size != n * sizeof(*res)) {
> -            syslog(LOG_ERR, "guest xorg resolution message has wrong size, "
> -                            "disconnecting agent");
> -            udscs_destroy_connection(connp);
> -            return;
> -        }
> +    g_free(agent_data->screen_info);
> +    agent_data->screen_info = g_memdup(data, header->size);

Code is fine either way,
Acked-by: Victor Toso <victortoso at redhat.com>

> +    agent_data->width  = header->arg1;
> +    agent_data->height = header->arg2;
> +    agent_data->screen_count = n;
>  
> -        g_free(agent_data->screen_info);
> -        res = g_malloc(n * sizeof(*res));
> -        memcpy(res, data, n * sizeof(*res));
> -        agent_data->width  = header->arg1;
> -        agent_data->height = header->arg2;
> -        agent_data->screen_info  = res;
> -        agent_data->screen_count = n;
> +    check_xorg_resolution();
> +}
>  
> -        check_xorg_resolution();
> +static void agent_read_complete(struct udscs_connection **connp,
> +    struct udscs_message_header *header, uint8_t *data)
> +{
> +    switch (header->type) {
> +    case VDAGENTD_GUEST_XORG_RESOLUTION:
> +        do_agent_xorg_resolution(connp, header, data);
>          break;
> -    }
>      case VDAGENTD_CLIPBOARD_GRAB:
>      case VDAGENTD_CLIPBOARD_REQUEST:
>      case VDAGENTD_CLIPBOARD_DATA:
> -- 
> 2.17.1
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180907/a0448ceb/attachment.sig>