[Xcb] Alignment problems in XKB GetGeometry

[Daniel Martin]

On 8 August 2013 09:48, Daniel Martin <consume.noise at gmail.com> wrote:
> On 6 August 2013 12:37, Ran Benita <ran234 at gmail.com> wrote:
>> On Tue, Aug 06, 2013 at 12:24:50AM +0200, Daniel Martin wrote:
>>> On Mon, Aug 05, 2013 at 10:45:09AM +0300, Ran Benita wrote:
>>> > On Mon, Aug 05, 2013 at 01:21:10AM +0200, Daniel Martin wrote:
>>> > > On Sun, Aug 04, 2013 at 11:57:10PM +0300, Ran Benita wrote:
>>> > So something's wrong here.
>>>
>>> It's XCB_TYPE_PAD(). In this case (sizeof(xcb_xkb_property_t)==0) it
>>> negates prev.index. I've attached a patch which changes XCB_TYPE_PAD to
>>> return 0 here. At least it makes your test programm (having Property
>>> with 2 CountedString16) to run without raising an assertion.
>>>
>>> I didn't made it a real git patch. Because, I think we've to add an
>>> XCB_ALIGN_PAD(), which would align based upon prev.data + prev.index and
>>> change the generated code to check the size of the type and use either
>>> XCB_TYPE_PAD or XCB_ALIGN_PAD. But, I'm not sure atm. if that's the
>>> correct solution. (Could be that prev.data + prev.index is already
>>> aligned.)
>>>
>>> diff --git a/src/xcb.h b/src/xcb.h
>>> index f7dc6af..45796f1 100644
>>> --- a/src/xcb.h
>>> +++ b/src/xcb.h
>>> @@ -87,7 +87,7 @@ extern "C" {
>>>  /** Connection closed because the server does not have a screen matching the display. */
>>>  #define XCB_CONN_CLOSED_INVALID_SCREEN 6
>>>
>>> -#define XCB_TYPE_PAD(T,I) (-(I) & (sizeof(T) > 4 ? 3 : sizeof(T) - 1))
>>> +#define XCB_TYPE_PAD(T,I) (sizeof(T) > 0 ? -(I) & (sizeof(T) > 4 ? 3 : sizeof(T) - 1) : 0)
>>>
>>>  /* Opaque structures */
>>>
>>
>> Thanks, with this patch I was able to test the other parts. Attached is
>> an updated test. The parts which rely on the xcb_xkb_doodad_t union size
>> TODO are commented out,
>
> Thanks, those tests help a lot.
>
>> I'm afraid I still don't know enough about the code generator to offer a
>> proper solution myself either.
>
> No one knows enough about the code generator. ;)
>
>> We need some other solution then the
>> empty struct though; it causes this compiler warning:
>>
>>     xkb.h:878:16: warning: struct has no members [-Wpedantic]
>>         typedef struct xcb_xkb_property_t {
>>         }
>>
>> And we probably can't rely on its sizeof() being 0. I'll try to figure
>> out why the other fix I tried didn't work.
>
> I've tried to address the problem yesterday. At least, I was able to generate
> the struct with having a "counted_string_foo name" as member. With that
> given the test programm passed the property checks ... to raise the next
> assert for colors. xcb_xkb_property_sizeof() became broken and therefor
> the start of colors was wrong, because the code generator didn't had a
> case before where the first and only member of a struct has a variadic size.

Didn't wanted to look at it yesterday, but I couldn't resist. I was
able to fix it up
until a TextDoodad occurs in the doodads list. Yet again a _sizeof() function
(xcb_xkb_text_doodad_sizeof) looks broken *sigh*.

btw. To traverse the doodads list you've to use the doodad specific iterator,
because the common iterator doesn't take the variadic length fields in the
specific doodad into account.