[Spice-devel] [spice-protocol PATCH v2 00/10] building with clang

[Frediano Ziglio]

> From: "Victor Toso" <lists at victortoso.com>
> 
> Hey,
> 
> On Fri, Aug 21, 2015 at 04:51:59AM -0400, Frediano Ziglio wrote:
> > Patches looks good.
> > There are only a small thing that puzzle me but if there are no objection
> > I'll ack them.
> >
> > Looking at code after the changes using CAST macros you can see something
> > (mostly the same for all occurrences)
> >
> >    *SPICE_ALIGNED_CAST(uint32_t, compressed_lines) = htonl(enc_size);
> >
> > Just looks strange that you there is a reference but not am explicit
> > pointer,
> > like
> >
> >    *SPICE_ALIGNED_CAST(uint32_t*, compressed_lines) = htonl(enc_size);
> >
> > I don't know probably it's just me.
> >
> > Frediano
> 
> I don't understand exaclty what you mean. If I understand correctly, you
> mean that SPICE_ALIGNED_CAST should be more explicit about returning a
> a pointer to the type?
> 
> Current define is:
> #define SPICE_ALIGNED_CAST(type, ptr)   ((type*)(void*)(ptr))
> 
> and if i understood correctly you would prefer:
> #define SPICE_ALIGNED_CAST(type, ptr)   ((type)(void*)(ptr))
> 
> Well, I really thought for some time about it but I wasn't sure either
> which way would be better but I agree that this way is more explicit.
> I'm fine with changing it. Do you want a v3?
> 
> cheers,
>   Victor Toso
> 

If nobody would complain for a couple of day I'll ack and push your patches.
It's just an impression.

Frediano

> >
> > > From: "Victor Toso" <lists at victortoso.com>
> > > 
> > > Hey,
> > > 
> > > Just to keep this update! I can send a v3 if requested ;)
> > > 
> > > pushed upstream:
> > > spice-protocol 01/10 macros: verify if __alloc_size__ works with clang
> > > spice-protocol 02/10 macros: fix alignment issue reported by clang
> > > spice-server 05/10 glz: WindowImageSegment lines lines_end as void*
> > > spice-server 06/10 mjpeg and jpeg encoder: fix alignment warnings
> > > spice-server 07/10 migration_protocol: use SPICE_MAGIC_CONST
> > > 
> > > need review:
> > > spice-protocol 03/10 macros: new define to help with aligment warnings
> > > spice-common 04/10 fix warnings about memory alignment
> > > spice-server 08/10 unaligned type with spice_marshaller_reserve_space
> > > spice-server 09/10 smartcard: use SPICE_ALIGNED_CAST
> > > spice-server 10/10 fix warnings about memory alignment
> > > 
> > > Cheers,
> > >  Victor Toso
> > > 
> > > On Fri, Aug 14, 2015 at 06:24:21PM +0200, Victor Toso wrote:
> > > > Here we go with v2. Maybe I could squash some of these patches...
> > > > Patch 0005 is acked-by Frediano;
> > > > Thanks for any input,
> > > > 
> > > > changes from v1
> > > > -> drop patch 02 and 12: on behaf of Frediano's better approach
> > > > (upstream)
> > > >    http://cgit.freedesktop.org/spice/spice-protocol/commit/?id=295d05e7330da45b705aa5081d140d7c84a931b0
> > > > 
> > > > -> Using SPICE_MAGIC_CONST on migration_protocol.h
> > > > 
> > > > -> patch 03 and 04: squashed (alignment for jpeg_encoder and
> > > > mjpeg_encoder)
> > > >    included Frediano's explanation on commit log and also a note that
> > > >    we
> > > >    consider VM's data being aligned
> > > > 
> > > > -> patch 05 and 06: acked and pushed
> > > > 
> > > > -> patch 08: reserve_space is unaligned - using Frediano suggestions by
> > > >    creating an unaligned type.
> > > > 
> > > > -> --enable-smartcard now + fixes
> > > > 
> > > > -> clang-noise patches are now using SPICE_ALIGNED_CAST and
> > > > SPICE_UNALIGNED_CAST
> > > >    to help track issues in the future.
> > > > 
> > > > Cheers,
> > > >   Victor Toso
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
>