[Xcb] [PATCH proto] xprint: Fix build of xprint extension

[Jaya Tiwari]

Hi Oliver,

I guess if we explicitly define pad align as a variable part before list,
we need to handle it in the end function, which is not handled currently I
guess.
(It is handled for functions accessing lists)

diff --git a/src/c_client.py b/src/c_client.py
index 9a7c67c..45ad2bf 100644
--- a/src/c_client.py
+++ b/src/c_client.py
@@ -1931,18 +1931,24 @@ def _c_accessors_list(self, field):
         if switch_obj is not None:
             _c('    i.data = %s + %s;', fields[field.c_field_name][0],
                _c_accessor_get_expr(field.type.expr, fields))
         elif field.prev_varsized_field == None:
             _c('    i.data = ((%s *) (R + 1)) + (%s);',
field.type.c_wiretype,
                _c_accessor_get_expr(field.type.expr, fields))
         else:
+            (prev_varsized_field, align_pad) = get_align_pad(field)
             _c('    xcb_generic_iterator_t child = %s;',
-               _c_iterator_get_end(field.prev_varsized_field, 'R'))
-            _c('    i.data = ((%s *) child.data) + (%s);',
field.type.c_wiretype,
-               _c_accessor_get_expr(field.type.expr, fields))
+                _c_iterator_get_end(prev_varsized_field, 'R'))
+            if align_pad is not None:
+                _c('    i.data = ((%s *) child.data + (%s) +
((-child.index) & (4 - 1)) + %d);', field.type.c_wiretype,
+                   _c_accessor_get_expr(field.type.expr,
fields),field.prev_varsized_offset)
+            else:
+               _c('    i.data = ((%s *) child.data) + (%s);',
field.type.c_wiretype,
+                   _c_accessor_get_expr(field.type.expr, fields))
+

         _c('    i.rem = 0;')
         _c('    i.index = (char *) i.data - (char *) %s;', param)
         _c('    return i;')
         _c('}')

     else:

With this, I could avoid the error.
But I am not sure of the solution though, maybe someone can come up with a
better handling for align pad list end accessors.

Also, just the implicit length lists needs to be the last member, so list
"options" can be kept as a length less list,
But then, as for request, we have accessor functions for every fields ,
options_len (implicit length of options list) also has an accessor function
named xcb_x_print_print_put_document_data_options_len !
So, instead of having an accessor for every field, we can have accessor for
just the wire fields but only ones that need accessor (i,e the variable
part of request)

diff --git a/src/c_client.py b/src/c_client.py
index 45ad2bf..3db313e 100644
--- a/src/c_client.py
+++ b/src/c_client.py
@@ -3097,15 +3097,22 @@ def c_request(self, name):
     else:
         # Request prototypes
         _c_request_helper(self, name, void=True, regular=False)
         _c_request_helper(self, name, void=True, regular=True)
         if self.c_need_aux:
             _c_request_helper(self, name, void=True, regular=False,
aux=True)
             _c_request_helper(self, name, void=True, regular=True,
aux=True)
-        _c_accessors(self, name, name)
+        for field in self.fields:
+            if not field.type.is_pad and field.wire:
+                print("My fields are : "+str(field.field_name))
+                if _c_field_needs_list_accessor(field):
+                    _c_accessors_list(self, field)
+                elif _c_field_needs_field_accessor(field):
+                    _c_accessors_field(self, field)
+

     # We generate the manpage afterwards because _c_type_setup has been
called.
     # TODO: what about aux helpers?
     _man_request(self, name, void=not self.reply, aux=False)

 def c_event(self, name):
     '''

This was working fine previously, because there was no padding between
lists of implicit length and a list having length. So this will solve the
issue of un necessary accessor functions.

Also, I am a little confused with the Implicit padding here.
For the lists with implicit length, it has an implicit padding added
because of the implicit length of list treated a variable sized field
before the list.

While, lists with a length field have their lengths as the fixed fields and
hence have no such padding issues.

Is this behavior expected?

Can you please correct me in my understanding if I am getting something
wrong here.

Thank you

Regards,
Jaya


On Tue, May 19, 2015 at 9:18 PM, Olivier Fourdan <ofourdan at redhat.com>
wrote:

> Hi Peter,
>
> > Since you're fixing the protocol definition of this request, you could
> > also change the <!-- padding --> comment to <pad align="4" />. ("pad
> > align" is relatively recent, and I'm guessing whoever originally wrote
> > xprint.xml was too lazy to put in the annoyingly large <op> block
> > required to do an alignment pad without "pad align").
>
> If I add the <pad align="4" /> statement, the generated code won't build
> anymore :(
>
> It fails with:
>
> xprint.c: In function 'xcb_x_print_print_put_document_data_doc_format_end':
> xprint.c:1338:36: error: 'None' undeclared (first use in this function)
>      xcb_generic_iterator_t child = None;
>                                     ^
> xprint.c:1338:36: note: each undeclared identifier is reported only once
> for each function it appears in
> xprint.c: In function 'xcb_x_print_print_put_document_data_options_end':
> xprint.c:1362:36: error: 'None' undeclared (first use in this function)
>      xcb_generic_iterator_t child = None;
>                                     ^
> Not sure why padding does that...
>
> > That said, this looks good. With or without the <pad align="4" />
> changes,
> > Reviewed-by: Peter Harris <pharris at opentext.com>
>
> Cheers!
> Olivier
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
>



-- 
Regards,
Jaya
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20150607/3a225c0e/attachment.html>