[Xcb] Alignment problems in XKB GetGeometry

[Daniel Martin]

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.
I'll continue with that fix during the next days. The code of the generator is
no fun, but with some beer it gets tolerable - the resulting code can't get
worse than the existing. ;)

Cheers,
    Daniel Martin