Bug 38331

Would be better without the ifdefs...

(In reply to comment #1)
> Would be better without the ifdefs...

The change just adds some unneeded space to align the structures to 8 bytes.  It should not cause additional problems if enabled on other architectures, except for increasing memory use by less than 8 bytes per each structure allocated in copy_classes.  I don't know any other architecture/ABI where such alignment is required but is not made without padding by structures having appropriate size.

Can this be addressed instead with an __attribute__ ((aligned (8))) somewhere?

(In reply to comment #3)
> Can this be addressed instead with an __attribute__ ((aligned (8))) somewhere?

All structures used are already aligned to 8 bytes, but the code puts there also arrays of 4 byte objects.  I don't see any other solution than manually padding it to start XIValuatorClassInfo structures at addresses aligned to 8 bytes.

> --- Comment #4 from Michał Masłowski <mtjm@mtjm.eu> 2011-10-12 07:56:34 PDT ---
> (In reply to comment #3)
> > Can this be addressed instead with an __attribute__ ((aligned (8))) somewhere?
> 
> All structures used are already aligned to 8 bytes, but the code puts there
> also arrays of 4 byte objects.  I don't see any other solution than manually
> padding it to start XIValuatorClassInfo structures at addresses aligned to 8
> bytes.
> 
Would there be a downside to doing that for all archs (or all 64bit
archs), rather than just one mips abi? (aside from 4-bytes holes here
and there)

(In reply to comment #5)
> > All structures used are already aligned to 8 bytes, but the code puts there
> > also arrays of 4 byte objects.  I don't see any other solution than manually
> > padding it to start XIValuatorClassInfo structures at addresses aligned to 8
> > bytes.
> > 
> Would there be a downside to doing that for all archs (or all 64bit
> archs), rather than just one mips abi? (aside from 4-bytes holes here
> and there)

There would be no other downside.  It probably is already 8 byte aligned on arches with 64-bit longs and pointers (N32 uses 32-bit longs and pointers, while long longs and doubles use native registers and need 8 byte alignment).

=/

Ok, would you please send your patch to xorg-devel for review?

07ced7b48219e3bc0c98806f3d7106f86d1b2ca0 added a pad_to_xid() to avoid 4-byte alignment of a 8-byte XID. afaict, your patch could be worked into this function to always align on 8 bytes on the mips.

a simple
#ifdef _ABIN32
  padsize = 8;
#endif

or somesuch in pad_to_xi2() should do, right?

The pad_to_xid change affects other architectures for an error in different part of the library.  I don't see a simpler solution for this bug than manually padding XIValuatorClassInfo on MIPS N32 in copy_classes, like in my patch (a modified version of which I have sent to the list).

This is a mass change of bugs. Bugs assigned to me that haven't been updated in the last 3 years are closed as WONTFIX, because, well, let's at least be honest about it.

Please do not re-open unless you have a really good reason to do so (e.g. you're fixing it yourself). If it hasn't been fixed in the last 3 years, it probably won't be fixed anytime soon either. Sorry.