[Xcb] progress with generator fixes for xkb ( Re: [PATCH proto xkb] Fixes for xkb.xml )

[Ran Benita]

On Fri, Feb 21, 2014 at 05:03:14PM +0100, Christian Linhart wrote:
> Hi again Ran,
> 
> I have worked on the problems of the generator with "xkb.xml + patches from my previous mail".
> 
> So far, I have identified the following three problems:
> 
> 1. The generator did not support structs which contain only variable-sized fields.
>     This has caused the empty struct xcb_xkb_property_t.
> 
>     I have made a first patch for c_client.py in libxcb to fix this, which is attached to this mail.
>     The patch makes xcb_xkb_property_t a "typedef void", and causes serialize and sizeof functions to be generated for xcb_xkb_property.
>     Also, the serialize function does not have an _aux parameter because it is not used in that case.
>     ( It is not used because there are no fixed size fields to read from the _aux parameter. )

I'm less familiar with the code generator, and it's pretty hard to read :(
However it'd be great to fix these things, so I'll try to go over your
changes. It might take some time...

But again if you provide some geomtest.c-like test which shows this
stuff actually starts to work that would be very helpful.

> 
> 2. When var-sized structs are passed to serialize functions ( or to request functions ),
>     then that function assumes that the struct is serialized, but it lists the struct type on the parameter list.
> 
>     for example:
> 
>         int
>         xcb_xkb_property_serialize (void                        **_buffer  /**< */,
>                             xcb_xkb_counted_string_16_t  *name  /**< */,
>                             xcb_xkb_counted_string_16_t  *value  /**< */);
> 
> 
>     It should list these parameters as "void*". ( like it is done with "switch" data when passed to such functions. )
> 
>     for example:
>         int
>         xcb_xkb_property_serialize (void                        **_buffer  /**< */,
>                             void  *name  /**< */,
>                             void  *value  /**< */);
> 

I'll have to get familiar with that, so can't provide any opinion yet.
How will the user-facing API will be like? A getter for the name
counted_string, and a getter for the value counted_string?
I don't think the user currently needs to use any _serialize() function
directly (only _unpack() in some rare cases, unfortunately).

>     Or we could make the code a bit more readable by using typedefs to void*, e.g.:
> 
>         typedef void* xcb_xkb_counted_string_16_serialized_t;
> 
>         [...]
> 
>         typedef void* xcb_xkb_property_serialized_t;
> 
>         [...]
> 
>         int
>         xcb_xkb_property_serialize (xcb_xkb_property_serialized_t                        *_buffer  /**< */,
>                             xcb_xkb_counted_string_16_serialized_t  *name  /**< */,
>                             xcb_xkb_counted_string_16_serialized_t  *value  /**< */);
> 
>     I have not yet made a patch for that.
> 
> 
> 3. Types like xcb_xkb_counted_string_16_t do not have a serialize function yet, but it would be needed in order to generate serialized data for such types.
> 
>     I have not yet made a patch for that, either.

It'd be nice if we don't have to iterate each char of the counted_string
individually, i.e. if we can get the length and get a char*. Don't know
if that's possible?

> 
> 
> So, this is work in progress.
> 
> I'd like to get your comments on that in order to check whether I am working in a good direction.

Sorry I can't help much here, most xcb developers are wary of this part
of the generator.

> Have a nice weekend,
> 
> Chris
> 
> P.S: Sorry that my previous email was formatted in HTML which the archiver converts to an attachment.
> This mail is in plain text ( if my mailer handles this correctly )

Yea it's fine. You might want to wrap to 72 chars (except for code and
patches) if possible.

Thanks for working on this!

Ran