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

[Victor Toso]

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

>
> > 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
> >