[Spice-devel] [PATCH spice-server v3] glz-encoder: Avoid double byte swap sending image magic

[Christophe Fergeau]

Ah thanks for expanding the commit log, still
Acked-by: Christophe Fergeau <cfergeau at redhat.com>

On Tue, Jun 05, 2018 at 09:57:25AM +0100, Frediano Ziglio wrote:
> encode_32 already deals with endian, don't swap twice.
> Tested with a ppc64 server machine and a x64 client.
> 
> This looks the reverse of a previous patch (59c6c82) supposed to fix big
> endian machine. encode_32 has been always:
> 
> static inline void encode_32(Encoder *encoder, unsigned int word)
> {
>     encode(encoder, (uint8_t)(word >> 24));
>     encode(encoder, (uint8_t)(word >> 16) & 0x0000ff);
>     encode(encoder, (uint8_t)(word >> 8) & 0x0000ff);
>     encode(encoder, (uint8_t)(word & 0x0000ff));
> }
> 
> while encode basically is similar to a putc on a FILE stream so is writing
> numbers from host endian to big endian order.
> The "main" endian (the one more tested since ever) is host/guest being
> little endian. So if you call encode_32 with a 0x01020304 you get 4 bytes
> in the order 1, 2, 3, 4.
> Before and after 59c6c82 LZ_MAGIC was defined as:
>   #define LZ_MAGIC (*(uint32_t *)"LZ  ")
> so on little endian this was 0x4c, 0x5a, 0x20, 0x20 that is 0x20205a4c
> which written through encode_32 become 0x20, 0x20, 0x5a, 0x4c so we can say
> that at the end on the network we must have 0x20, 0x20, 0x5a, 0x4c.
> On big endian however LZ_MAGIC got the value 0x4c5a2020 which written
> through encode_32 get 0x4c, 0x5a, 0x20, 0x20 which is the opposite
> expected. So patch 59c6c82 reverted the order having again 0x20, 0x20,
> 0x5a, 0x4c on the network.
> However commit 5a7e587 (spice-common), in an attempt to avoid double
> swapping on LZ, changed LZ_MAGIC to
>   #define LZ_MAGIC 0x20205a4c
> breaking endianness again for GLZ code.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/glz-encoder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Changes since v2:
> - fix a byte order. Yes, is that confusing :-)
> 
> Changes since v1:
> - extend commit message.
> 
> diff --git a/server/glz-encoder.c b/server/glz-encoder.c
> index dba2cd12..3ad2b97c 100644
> --- a/server/glz-encoder.c
> +++ b/server/glz-encoder.c
> @@ -262,7 +262,7 @@ int glz_encode(GlzEncoderContext *opaque_encoder,
>      encoder->cur_image.id = dict_image->id;
>      encoder->cur_image.first_win_seg = dict_image->first_seg;
>  
> -    encode_32(encoder, GUINT32_TO_LE(LZ_MAGIC));
> +    encode_32(encoder, LZ_MAGIC);
>      encode_32(encoder, LZ_VERSION);
>      if (top_down) {
>          encode(encoder, (type & LZ_IMAGE_TYPE_MASK) | (1 << LZ_IMAGE_TYPE_LOG));
> -- 
> 2.17.1
> 
> _______________________________________________
> 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: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180605/fa2b20f7/attachment.sig>