[Spice-devel] [spice-gtk][PATCH 3/4] ppc: Fix message endianess

[Fabiano Fidêncio]

On Mon, May 18, 2015 at 6:55 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> On Mon, May 18, 2015 at 03:11:07PM +0200, Fabiano Fidêncio wrote:
>> ---
>>  gtk/spice-channel.c | 43 +++++++++++++++++++++----------------------
>>  1 file changed, 21 insertions(+), 22 deletions(-)
>>
>> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
>> index e7460a6..0da377b 100644
>> --- a/gtk/spice-channel.c
>> +++ b/gtk/spice-channel.c
>> @@ -1149,15 +1149,14 @@ static void spice_channel_send_link(SpiceChannel *channel)
>>      c->link_hdr.major_version = GUINT32_TO_LE(c->link_hdr.major_version);
>>      c->link_hdr.minor_version = GUINT32_TO_LE(c->link_hdr.minor_version);
>>
>> -    c->link_msg.connection_id = spice_session_get_connection_id(c->session);
>> +    c->link_msg.connection_id = GUINT32_TO_LE(spice_session_get_connection_id(c->session));
>>      c->link_msg.channel_type  = c->channel_type;
>>      c->link_msg.channel_id    = c->channel_id;
>> -    c->link_msg.caps_offset   = sizeof(c->link_msg);
>> +    c->link_msg.caps_offset   = GUINT32_TO_LE(sizeof(c->link_msg));
>>
>> -    c->link_msg.num_common_caps = c->common_caps->len;
>> -    c->link_msg.num_channel_caps = c->caps->len;
>> -    c->link_hdr.size += (c->link_msg.num_common_caps +
>> -                         c->link_msg.num_channel_caps) * sizeof(uint32_t);
>> +    c->link_msg.num_common_caps = GUINT32_TO_LE(c->common_caps->len);
>> +    c->link_msg.num_channel_caps = GUINT32_TO_LE(c->caps->len);
>> +    c->link_hdr.size += (c->common_caps->len + c->caps->len) * sizeof(uint32_t);
>>
>>      buffer = g_malloc0(sizeof(c->link_hdr) + c->link_hdr.size);
>>      p = buffer;
>> @@ -1168,18 +1167,18 @@ static void spice_channel_send_link(SpiceChannel *channel)
>>      memcpy(p, &c->link_msg, sizeof(c->link_msg)); p += sizeof(c->link_msg);
>>
>>      for (i = 0; i < c->common_caps->len; i++) {
>> -        *(uint32_t *)p = g_array_index(c->common_caps, uint32_t, i);
>> +        *(uint32_t *)p = GUINT32_TO_LE(g_array_index(c->common_caps, uint32_t, i));
>>          p += sizeof(uint32_t);
>>      }
>>      for (i = 0; i < c->caps->len; i++) {
>> -        *(uint32_t *)p = g_array_index(c->caps, uint32_t, i);
>> +        *(uint32_t *)p = GUINT32_TO_LE(g_array_index(c->caps, uint32_t, i));
>>          p += sizeof(uint32_t);
>>      }
>>      CHANNEL_DEBUG(channel, "channel type %d id %d num common caps %d num caps %d",
>> -                  c->link_msg.channel_type,
>> -                  c->link_msg.channel_id,
>> -                  c->link_msg.num_common_caps,
>> -                  c->link_msg.num_channel_caps);
>> +                  c->channel_type,
>> +                  c->channel_id,
>> +                  c->common_caps->len,
>> +                  c->caps->len);
>>      spice_channel_write(channel, buffer, p - buffer);
>>      g_free(buffer);
>>  }
>> @@ -1721,24 +1720,24 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
>>          goto error;
>>      }
>>
>> -    num_caps = c->peer_msg->num_channel_caps + c->peer_msg->num_common_caps;
>> +    num_caps = GUINT32_FROM_LE(c->peer_msg->num_channel_caps + c->peer_msg->num_common_caps);
>
> You should not do the addition on values not in the native endianness.
> This should be:
> num_caps = GUINT32_FROM_LE(c->peer_msg->num_channel_caps) + GUINT32_FROM_LE(c->peer_msg->num_common_caps);
> Given that you reuse both num_channel_caps and num_common_caps several
> times, I'd add some local variables to improve readability.

Okay, I will change it.
Thanks for the review.

>
> Looks good otherwise, ACK.
>
> Christophe
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Fabiano Fidêncio