[Spice-devel] [PATCH spice-gtk 2/2] spice-channel: Factor out code for storing caps

[Christophe Fergeau]

On Thu, Jun 29, 2017 at 12:38:36PM +0200, Pavel Grunt wrote:
> Signed-off-by: Pavel Grunt <pgrunt at redhat.com>
> ---
>  src/spice-channel.c | 41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index b8cf19c..2422811 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -1884,11 +1884,32 @@ cleanup:
>  #endif /* HAVE_SASL */
>  
>  /* coroutine context */
> +static void store_remote_caps(SpiceChannel *channel,
> +                              const bool are_common,

I think I would replace this with a GArray *caps_dest (which would be
either c->remote_common_caps or c->remote_caps), and infer the debug
string from that (yeah, not that great either). Or pass both as
parameters, and have store_remote_caps/store_remote_common_caps helpers.
But this is getting quite overengineered for a simple function ;)

Acked-by: Christophe Fergeau <cfergeau at redhat.com>
feel free to follow one of these suggestions, or to just push this.

Christophe

> +                              const uint8_t *caps_src,
> +                              const guint num_caps)
> +{
> +    SpiceChannelPrivate *c = channel->priv;
> +    uint32_t *caps;
> +    guint i;
> +    GArray *remote_caps = are_common ? c->remote_common_caps : c->remote_caps;
> +    const gchar *caps_name = are_common ? "common" : "channel";
> +
> +    g_array_set_size(remote_caps, num_caps);
> +    caps = &g_array_index(remote_caps, uint32_t, 0);
> +    memcpy(caps, caps_src, num_caps * sizeof(uint32_t));
> +    for (i = 0; i < num_caps; i++, caps++) {
> +        *caps = GUINT32_FROM_LE(*caps);
> +        CHANNEL_DEBUG(channel, "got %s caps %u:0x%X", caps_name,  i, *caps);
> +    }
> +}
> +
> +/* coroutine context */
>  static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
>  {
>      SpiceChannelPrivate *c;
> -    int rc, num_caps, i;
> -    uint32_t *caps, num_channel_caps, num_common_caps;
> +    int rc, num_caps;
> +    uint32_t num_channel_caps, num_common_caps;
>      uint8_t *caps_src;
>      SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK;
>  
> @@ -1930,22 +1951,10 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
>      /* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */
>  
>      caps_src = (uint8_t *)c->peer_msg + GUINT32_FROM_LE(c->peer_msg->caps_offset);
> -    g_array_set_size(c->remote_common_caps, num_common_caps);
> -    caps = &g_array_index(c->remote_common_caps, uint32_t, 0);
> -    memcpy(caps, caps_src, num_common_caps * sizeof(uint32_t));
> -    for (i = 0; i < num_common_caps; i++, caps++) {
> -        *caps = GUINT32_FROM_LE(*caps);
> -        CHANNEL_DEBUG(channel, "got common caps %d:0x%X", i, *caps);
> -    }
> +    store_remote_caps(channel, true, caps_src, num_common_caps);
>  
>      caps_src += num_common_caps * sizeof(uint32_t);
> -    g_array_set_size(c->remote_caps, num_channel_caps);
> -    caps = &g_array_index(c->remote_caps, uint32_t, 0);
> -    memcpy(caps, caps_src, num_channel_caps * sizeof(uint32_t));
> -    for (i = 0; i < num_channel_caps; i++, caps++) {
> -        *caps = GUINT32_FROM_LE(*caps);
> -        CHANNEL_DEBUG(channel, "got channel caps %d:0x%X", i, *caps);
> -    }
> +    store_remote_caps(channel, false, caps_src, num_channel_caps);
>  
>      if (!spice_channel_test_common_capability(channel,
>              SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION)) {
> -- 
> 2.13.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170629/90220016/attachment.sig>