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

[Frediano Ziglio]

> 
> > 
> > On Mon, Jun 04, 2018 at 10:30:12AM +0100, Frediano Ziglio wrote:
> > > encode_32 already deals with endian, don't swap twice.
> > > Tested with a ppc64 server machine and a x64 client.
> > > 
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  server/glz-encoder.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > 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);
> > 
> > What is interesting is that this was changed to fix some endianness
> > issues in 59c6c82, and encode_32 definition hasn't changed since then...
> > But I indeed agree that swapping should not be necessary.
> > 
> > Christophe
> > 
> 
> First: I tested it.
> Second: if this patch were wrong also you would need to reverse all other
>   fields (width/height, version and so on).
> 
> I'll put the author name on my list of people that do not test the
> patches they send :-)
> 

Some digging, code was

./spice-common/common/lz.c:    encode_32(encoder, GUINT32_TO_LE(LZ_MAGIC));
./spice-common/common/lz.c:    if (magic != LZ_MAGIC) {
./spice-common/common/lz_common.h:#define LZ_MAGIC (*(uint32_t *)"LZ  ")
./server/glz_encoder.c:    encode_32(encoder, GUINT32_TO_LE(LZ_MAGIC));

now

./spice-common/common/lz.c:    encode_32(encoder, LZ_MAGIC);
./spice-common/common/lz.c:    if (magic != LZ_MAGIC) {
./spice-common/common/lz_common.h:#define LZ_MAGIC 0x20205a4c
./server/glz-encoder.c:    encode_32(encoder, LZ_MAGIC);

so both commit 59c6c82 and this patch are correct.
At the time of commit 59c6c82 LZ_MAGIC was depending on endianness while now is not
(always little endian).

Frediano