[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Upstream asks: unportable patch for mirrormagic?



Hello Michel,

> > > > -       gi->checked = va_arg(ap, boolean);
> > > > +       gi->checked = (boolean) va_arg(ap, int);
> > >
> > > Hm, applying this patch would mess up things for other platforms,
> > > because "boolean" is typedef'ed as "unsigned char". If it simply
> > > gets replaced by "int", va_arg reads a 16 or 32 bit value instead
> > > of a 8 bit value.
> 
> So is that value really only 8 bits

That value (boolean) is typedef'ed to "unsigned char", which
normally is only 8 bits, unless your preferred C compiler
automatically transforms this to another type/size. It seems
that exactly this was the cause for the problem here -- see below.

> (why doesn't it work out of the box on powerpc then?),

That's something _you_ should know better than I. ;-)

> or does the code wrongly assume that the first byte
> of a larger value is the lowest significant one?

Of course not! :-)

The (variadic) function gets an "unsigned char" as an argument,
so I guessed to read it back (with va_arg) also as an "unsigned
char". This was a wrong assumption, as Lukas Geyer made clear in
his mail (also on this mail thread), and I in fact have to read
it with va_arg(ap, int). Maybe it's a good idea to generally
define (with typedef) "boolean" as "int" -- nowadays there is
not speedup with using 8 bit instead of 16/32/whatever bit for a
datatype anyway. ;-)

Thanks for your comments and help!

Best regards,
		Holger
-- 
  // Holger Schemel        |
\X/  aeglos@valinor.owl.de | Microsoft -- confusion at your fingertips.



Reply to: