[Spice-devel] [PATCH] reds: send the number of channel in the LE byte-order

[Denis Kirjanov]

On 3/27/15, Christophe Fergeau <cfergeau at redhat.com> wrote:
> Hey,
>
> On Thu, Mar 26, 2015 at 05:52:05PM +0300, Denis Kirjanov wrote:
>> Since the spice protocol is LE send the number of channels in
>> the proper byte order
>>
>> Signed-off-by: Denis Kirjanov <kda at linux-powerpc.org>
>> ---
>>  server/reds.c |    4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/server/reds.c b/server/reds.c
>> index f61d5d3..468ac9c 100644
>> --- a/server/reds.c
>> +++ b/server/reds.c
>> @@ -37,6 +37,8 @@
>>  #include <ctype.h>
>>  #include <stdbool.h>
>>
>> +#include <glib.h>
>> +
>>  #include <openssl/err.h>
>>
>>  #if HAVE_SASL
>> @@ -874,7 +876,7 @@ void reds_fill_channels(SpiceMsgChannels
>> *channels_info)
>>          used_channels++;
>>      }
>>
>> -    channels_info->num_of_channels = used_channels;
>> +    channels_info->num_of_channels = GUINT32_TO_LE(used_channels);
>
> Are you sure this is needed? In other words, what concrete bug are you
> observing without that change?
> reds_fill_channels is called from main_channel_marshall_channels(),
> which then passes the filled SpiceMsgChannels instance to
> spice_marshall_msg_main_channels_list()
>
> spice_marshall_msg_* is generated from the description of the
> SPICE protocol available in spice.proto/spice1.proto. These generated
> marshallers are usually correct from an endianness point of view.
>
> This specific function is defined in
> spice-common/common/generated_server_marshallers.c and does:
>
> spice_marshaller_add_uint32(m, src->num_of_channels);
>
> spice_marshaller_add_uint32() calls spice_marshaller_add_uint32() which
> is
> #ifdef WORDS_BIGENDIAN
> #define write_uint32(ptr,v) (*((uint32_t *)(ptr)) =
> SPICE_BYTESWAP32((uint32_t)(v)))
> #else
> #define write_uint32(ptr,v) (*((uint32_t *)(ptr)) = v)
> #endif
>
> so I would say the existing code is correct, and this patch unneeded.

The problem is that write_uint32t writes the value in the BE even if
the client is LE. Obviously it's wrong.


> Christophe
>


-- 
Regards,
Denis