Hi Adrian, On Fri, Jun 22, 2018 at 1:00 PM John Paul Adrian Glaubitz <glaub...@physik.fu-berlin.de> wrote: > On 06/22/2018 12:54 PM, Geert Uytterhoeven wrote: > >>> If anything relies on having a hole in between the two fields, explicit > >>> padding should be added. > >> > >> Well, I don't think we can add this change as this reduces the size of > >> the bitfield. Do you think we can just cut it in half without any issues? > > > > CSSProperty contains: > > > > StylePropertyMetadata m_metadata; > > RefPtr<CSSValue> m_value; > > > > StylePropertyMetadata contains: > > uint16_t m_propertyID : 10; > > uint16_t m_isSetFromShorthand : 1; > > uint16_t m_indexInShorthandsVector : 2; // If this property was > > set as part of an ambiguous shorthand, gives the index in the > > shorthands vector. > > uint16_t m_important : 1; > > uint16_t m_implicit : 1; // Whether or not the property was set > > implicitly as the result of a shorthand. > > uint16_t m_inherited : 1; > > uint16_t pad; > > > > So the bitfield is in the first uint16_t, not in a uint32_t (yes, that > > matters > > on big endian). > > But our problem is not endianess-related, is it? Our problem is that the > default alignment on m68k is 16 bits.
Yes, that's what the "<=" checks are for. > > Oops, I did miss StylePropertyMetadata already has padding. > > So the sizes of CSSProperty and SameSizeAsCSSProperty should already match. But the "==" check should be fine, at least for the first structure, as someone has already made the padding explicit (which is IMHO a better solution than changing "==" to "<="). > > To avoid future endianness issues, you still may want to reflect this in > > SameSizeAsCSSProperty: > > > > --- webkitgtk-2.20.3/Source/WebCore/css/CSSProperty.cpp.orig > > 2018-06-22 10:40:49.389060321 +0200 > > +++ webkitgtk-2.20.3/Source/WebCore/css/CSSProperty.cpp 2018-06-22 > > 12:50:51.005024268 +0200 > > @@ -29,7 +29,8 @@ > > namespace WebCore { > > > > struct SameSizeAsCSSProperty { > > - uint32_t bitfields; > > + uint16_t bitfields; > > + uint16_t pad; > > void* value; > > }; > > I'm confused. The endinaness is not our problem at the moment, is it? Not at the moment. But it may bite us in the future. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds