[Spice-devel] [PATCH] change marshalling macros to consider alignment of types

[Frediano Ziglio]

> 
> It doesn't work.
> 

Short reply:
It works for me.

Long reply:
I tested this code and "seems to work" for me.
This code tell the gcc compiler that these integers (on little endian) are not aligned on memory.
This with the -mno-unaligned-access "seems to work".
"seems to work" in my case means that with the protocol change, an ARM gcc compiler and
the option enabled the code don't produce unaligned access or better produce assembly
code which does not unaligned access (didn't run the code).
Did you use this option too or just the patch?
Are you sometimes on IRC to have a faster round trip than e-mail?
Can you compile with "make V=1" and give some output with full command lines to check for
-mno-unaligned-access?
Are you sure you are using newer protocol version and not an old one?

Frediano


> 11.12.2015, 17:22, "Frediano Ziglio" <fziglio at redhat.com>:
> >>  More details in the log as to when this is needed would be useful.
> >>
> >>  On Fri, Dec 11, 2015 at 01:36:41PM +0000, Frediano Ziglio wrote:
> >>  > ---
> >>  > python_modules/demarshal.py | 30 ++++++++++++++++++++++--------
> >>  > 1 file changed, 22 insertions(+), 8 deletions(-)
> >>  >
> >>  > diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
> >>  > index 209eafc..743b0d8 100644
> >>  > --- a/python_modules/demarshal.py
> >>  > +++ b/python_modules/demarshal.py
> >>  > @@ -40,20 +40,34 @@ def write_parser_helpers(writer):
> >>  >
> >>  > writer = writer.function_helper()
> >>  >
> >>  > + for sign in ["", "u"]:
> >>  > + type = "%sint8" % sign
> >>  > + writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" % type)
> >>  > + writer.macro("write_%s" % type, "ptr, val", "(*((%s_t *)(ptr))) =
> >>  > val" % type)
> >>  > writer.writeln("#ifdef WORDS_BIGENDIAN")
> >>  > - for size in [8, 16, 32, 64]:
> >>  > + for size in [16, 32, 64]:
> >>
> >>  I don't think this hunk is strictly related to the alignment change, it
> >>  could go in a "Don't wrap 8 bit accesses in unneeded #ifdef
> >>  WORDS_BIGENDIAN"
> >
> > Yes, just helps reducing code generated.
> >
> >>  > for sign in ["", "u"]:
> >>  > utype = "uint%d" % (size)
> >>  > type = "%sint%d" % (sign, size)
> >>  > swap = "SPICE_BYTESWAP%d" % size
> >>  > - if size == 8:
> >>  > - writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))"
> >>  > % type)
> >>  > - writer.macro("write_%s" % type, "ptr, val", "*(%s_t
> >>  > *)(ptr) = val" % (type))
> >>  > - else:
> >>  > - writer.macro("read_%s" % type, "ptr", "((%s_t)%s(*((%s_t
> >>  > *)(ptr))))" % (type, swap, utype))
> >>  > - writer.macro("write_%s" % type, "ptr, val", "*(%s_t
> >>  > *)(ptr) = %s((%s_t)val)" % (utype, swap, utype))
> >>  > + writer.macro("read_%s" % type, "ptr", "((%s_t)%s(*((%s_t
> >>  > *)(ptr))))" % (type, swap, utype))
> >>  > + writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr) =
> >>  > %s((%s_t)val)" % (utype, swap, utype))
> >>  > + writer.writeln("#elif __GNUC__ > 3")
> >>  > + struct = "pkg_struct"
> >>
> >>  Why is it called 'pkg_struct' ?
> >
> > Was a struct before... and few fantasy at the moment :)
> >
> >>  I'll let Anton tell us if this helps with his situation or not before
> >>  looking closer at that patch
> >>
> >>  Christophe
> >
> > I should have put an "RFC", this patch was intended to be tested by Anton
> > (it's in reply to Anton's thread).
> >
> > Note that this patch does not consider architectures with big endian and
> > unaligned access problems.
> >
> > Frediano
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> --
> Anton D. Kachalov
> 
> ITO, System Architect
> Tel: 7 (495) 739-70-00 ext.7613
>