[Xcb] [PATCH proto 1/1 RESEND] Calculate length of lengthless lists

[Ran Benita]

On Wed, Apr 01, 2015 at 11:31:00PM +0530, Jaya Tiwari wrote:
> Hi Ran,

Hi Jaya, sorry for not replying sooner.

> Thankyou for the comments and sorry for not including the context here.
> Have a brief context included below now.
> 
> In current XCB, the length of the rectangles list of request
> SetPictureClipRectangles
> is not specified in render.xml:
>   <request name="
> SetPictureClipRectangles" opcode="6">
>     <field type="PICTURE" name="picture" />
>     <field type="INT16" name="clip_x_origin" />
>     <field type="INT16" name="clip_y_origin" />
>     <list type="RECTANGLE" name="rectangles" />
>   </request>
> 
> This needs to be fixed for some applications, including server-side XCB.
> So, the xml will be changed as follows, by using the length-field of
> the request to compute the length of the rectangles-list.
> 
> So here, such length less lists are identified and are then generator
> generates a suitable length expressions for such identified lists from
> previous passes.

Thanks for the context. So just to make sure I got it right:

The length of the rectangles list is calculated by
    (<total request len> - sizeof(<base request>)) / sizeof(<rectange struct>)

For the client-side xcb the client supplies the `rectangles_len`, so
there is no problem and the request is constructed correctly.

I have not followed the server-side xcb discussion, but I assume there
the role of requests and replies is inverted. So you need to reconstruct
the request above from a byte-stream. And then you need to supply a
`_length()` function for the list as usual.

That makes sense to me. Since I don't know how to test the server-side
XCB, can you say XCB does with above request without your patches? Does
it fail or output bad code or...?

> 1 . I have changed the following things suggested by you :
> > +    def __init__(self, elt, member, no_of_var_fields, *parent):
> "num" is a more common acronym for "number".
> 
> Changed to num.
> 
> 2. Actually, I don't see `dont_have_len`, `has_lenfield_ref` and
> `lenfield_name` used anywhere in this function, why add them?
> 
> Yes, I had added them previously for more test cases, but later
> discovered they are no longer needed. Sorry for not removing them
> earlier, removed now.
> 
> 3. if 'Request' in str(self.parents):
> This test is not very robust, please use a proper test here instead of
> relying on name pretty-printing.
> 
> Actually I have to just make sure that the lengthless lists have
> length calculated for requests only, and no other container.
> 
> 4. > +            if not len(elts) and has_request and no_of_var_fields == 1:
> Please change `not len(elts)` to `len(elts) == 0`; `not` should be
> reserved for boolean IMO.
> 
> Yes, Thankyou for pointing it out. Changed this one too.
> 
> Can you please suggest some solution for request checking too, is
> there any way I can check the tag of parent in ListType, or check and
> pass it from resolve of ComplexType ?
> Meanwhile, I am trying to figure out a workaround too, and when done,
> will post a patch with your mentioned corrections and will take care
> while resending the patch.

I'll look at your patches and reply there.

> Thankyou for helping out.
> 
> Regards,
> Jaya