[Xcb] [PATCH libxcb V2] generator: support parametrized structs

Christian Linhart chris at DemoRecorder.com
Wed Oct 22 06:34:01 PDT 2014


Parametrized structs contain paramref expressions which
refer to the value of a field defined in the context
where the struct is used.

Implementing the parametrized structs turned out
to be somewhat easier than previously thought
because the generator already had some support for type-parametrization
because this is needed when case or bitcase refers to fields outside
of the switch.

So I decided to go with the flow and to implement the solution
which best fits the current implementation.

I did the following:
* I provided a way to specify fieldref with an explicitely given type:
  This resulted in <paramref type="CARD8>fieldname</paramref>
  A paramref is just a fieldref with an explicit type.
  The type is necessary because there is no local field of that
  name where the type can be derived from.

* then I tested it and made several changes in the generator
  such that it really works.

Basically the generated code is as follows:
* The parameter appears on the parameter list of the
  sizeof-function of the parametrized struct.
  When that function gets called, an appropriate argument is supplied.

* The parameter also appears as an additional member of the iterator-struct
  for the iterator of lists of that parametrized struct.
  This way, the next-function can get the value of that parameter from the iterator.
  When the iterator is created, this iterator-member is set accordingly.

* When the paramref appears in the length-expression of a list, then
  the parameter appears on the parameterlist of the "length" and "end" functions.
  When these functions get called, an appropriate argument is supplied.

Some comments:
* I did not implement inline structs.
  This would probably have been more complicated, and at least some additional effort.
  But that can be implemented later if needed.
  (Inline structs could probably use some code from switch-case/bitcase which is already kind of
  an inlined struct but one has to be careful not to break the functionality
  of switch-case/bitcase. Support for inline structs inside lists must probably
  be implemented from scratch...)

* The paramref expression refers to a field of the same name in the struct/request/...
  where it is used.
  So it is not possible to pass the value of arbitrary fields or even expressions
  to the parametrized struct.
  This would have been possible with the previously discussed <typearg>.
  That can be added later, if needed.
  ( Wont be too complicated )

* So this is pretty much like the proposal from Ran Benita.

changes for V2 of this patch, according to suggestions from Ran Benita:
* replace map with list comprehension 
  because map returns an iterator instead of a list from Python 3 on,
  so it cannot be added to a list anymore.

* removed "self" parameter of function additional_params_to_str
  and accessed the variable additional_params from the outer 
  function directly.
  

---
 src/c_client.py | 130 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 110 insertions(+), 20 deletions(-)

diff --git a/src/c_client.py b/src/c_client.py
index fa0d2fc..cddc680 100644
--- a/src/c_client.py
+++ b/src/c_client.py
@@ -702,14 +702,39 @@ def resolve_expr_fields(complex_obj):
     # try to resolve expr fields
     for e in expr_fields:
         if e not in all_fields and e not in unresolved:
             unresolved.append(e)
     return unresolved
 # resolve_expr_fields()
 
+def resolve_expr_fields_list(self, parents):
+    """
+    Find expr fields appearing in a list and descendents
+    that cannot be resolved within the parents of the list.
+    These are normally fields that need to be given as function parameters
+    for length and iterator functions.
+    """
+    all_fields = []
+    expr_fields = get_expr_fields(self)
+    unresolved = []
+
+    for complex_obj in parents:
+        for field in complex_obj.fields:
+            if field.wire:
+                all_fields.append(field)
+
+    # try to resolve expr fields
+    for e in expr_fields:
+        if e not in all_fields and e not in unresolved:
+            unresolved.append(e)
+
+    return unresolved
+# resolve_expr_fields_list()
+
+
 def get_serialize_params(context, self, buffer_var='_buffer', aux_var='_aux'):
     """
     functions like _serialize(), _unserialize(), and _unpack() sometimes need additional parameters:
     E.g. in order to unpack switch, extra parameters might be needed to evaluate the switch
     expression. This function tries to resolve all fields within a structure, and returns the
     unresolved fields as the list of external parameters.
     """
@@ -906,14 +931,25 @@ def _c_serialize_helper_switch_field(context, self, field, c_switch_variable, pr
         #remove trailing ", " from c_field_names because it will be used at end of arglist
         my_c_field_names = c_field_names[:-2]
         length = "%s( xcb_tmp, %s )" % ( field.type.c_sizeof_name, my_c_field_names )
 
     return length
 # _c_serialize_helper_switch_field()
 
+def _c_get_additional_type_params(self):
+    """
+    compute list of additional params for functions created for the given type
+    """
+    if self.is_simple:
+        return []
+    else:
+        param_fields, wire_fields, params = get_serialize_params('sizeof', self)
+        return params[1:]
+# _c_get_additional_type_params()
+
 def _c_serialize_helper_list_field(context, self, field,
                                    code_lines, temp_vars,
                                    space, prefix):
     """
     helper function to cope with lists of variable length
     """
     expr = field.type.expr
@@ -944,36 +980,44 @@ def _c_serialize_helper_list_field(context, self, field,
     list_length = _c_accessor_get_expr(expr, field_mapping)
 
     # default: list with fixed size elements
     length = '%s * sizeof(%s)' % (list_length, field.type.member.c_wiretype)
 
     # list with variable-sized elements
     if not field.type.member.fixed_size():
+        # compute string for argumentlist for member-type functions
+        member_params = _c_get_additional_type_params(field.type.member)
+        member_arg_names = [p[2] for p in member_params]
+        member_arg_str = ''
+        for member_arg_name in member_arg_names:
+            member_arg_str += ', ' + field_mapping[member_arg_name][0]
+
+        #
         length = ''
         if context in ('unserialize', 'sizeof', 'unpack'):
             int_i = '    unsigned int i;'
             xcb_tmp_len = '    unsigned int xcb_tmp_len;'
             if int_i not in temp_vars:
                 temp_vars.append(int_i)
             if xcb_tmp_len not in temp_vars:
                 temp_vars.append(xcb_tmp_len)
             # loop over all list elements and call sizeof repeatedly
             # this should be a bit faster than using the iterators
             code_lines.append("%s    for(i=0; i<%s; i++) {" % (space, list_length))
-            code_lines.append("%s        xcb_tmp_len = %s(xcb_tmp);" %
-                              (space, field.type.c_sizeof_name))
+            code_lines.append("%s        xcb_tmp_len = %s(xcb_tmp%s);" %
+                              (space, field.type.c_sizeof_name, member_arg_str))
             code_lines.append("%s        xcb_block_len += xcb_tmp_len;" % space)
             code_lines.append("%s        xcb_tmp += xcb_tmp_len;" % space)
             code_lines.append("%s    }" % space)
 
         elif 'serialize' == context:
             code_lines.append('%s    xcb_parts[xcb_parts_idx].iov_len = 0;' % space)
             code_lines.append('%s    xcb_tmp = (char *) %s%s;' % (space, prefix_str, field.c_field_name))
             code_lines.append('%s    for(i=0; i<%s; i++) { ' % (space, list_length))
-            code_lines.append('%s        xcb_block_len = %s(xcb_tmp);' % (space, field.type.c_sizeof_name))
+            code_lines.append('%s        xcb_block_len = %s(xcb_tmp%s);' % (space, field.type.c_sizeof_name, member_arg_str))
             code_lines.append('%s        xcb_parts[xcb_parts_idx].iov_len += xcb_block_len;' % space)
             code_lines.append('%s    }' % space)
             code_lines.append('%s    xcb_block_len = xcb_parts[xcb_parts_idx].iov_len;' % space)
 
     return length
 # _c_serialize_helper_list_field()
 
@@ -1478,14 +1522,22 @@ def _c_iterator(self, name):
     _h('/**')
     _h(' * @brief %s', self.c_iterator_type)
     _h(' **/')
     _h('typedef struct %s {', self.c_iterator_type)
     _h('    %s *data; /**<  */', self.c_type)
     _h('    int%s rem; /**<  */', ' ' * (len(self.c_type) - 2))
     _h('    int%s index; /**<  */', ' ' * (len(self.c_type) - 2))
+    # add additional params of the type "self" as fields to the iterator struct
+    # so that they can be passed to the sizeof-function by the iterator's next-function
+    params = _c_get_additional_type_params(self)
+    for param in params:
+        _h('    %s%s %s; /**<  */',
+            param[0],
+            ' ' * (len(self.c_type) + 1 - len(param[0])),
+            param[2])
     _h('} %s;', self.c_iterator_type)
 
     _h_setlevel(1)
     _c_setlevel(1)
     _h('')
     _h('/**')
     _h(' * Get the next element of the iterator')
@@ -1505,17 +1557,22 @@ def _c_iterator(self, name):
         _c('    %s *R = i->data;', self.c_type)
 
         if self.is_union:
             # FIXME - how to determine the size of a variable size union??
             _c('    /* FIXME - determine the size of the union %s */', self.c_type)
         else:
             if self.c_need_sizeof:
+                # compute the string of additional arguments for the sizeof-function
+                additional_args = ''
+                for param in params:
+                    additional_args += ', i->' + param[2]
+
                 _c('    xcb_generic_iterator_t child;')
-                _c('    child.data = (%s *)(((char *)R) + %s(R));',
-                   self.c_type, self.c_sizeof_name)
+                _c('    child.data = (%s *)(((char *)R) + %s(R%s));',
+                   self.c_type, self.c_sizeof_name, additional_args)
                 _c('    i->index = (char *) child.data - (char *) i->data;')
             else:
                 _c('    xcb_generic_iterator_t child = %s;', _c_iterator_get_end(self.last_varsized_field, 'R'))
                 _c('    i->index = child.index;')
             _c('    --i->rem;')
             _c('    i->data = (%s *) child.data;', self.c_type)
 
@@ -1826,14 +1883,37 @@ def _c_accessors_list(self, field):
             if not p.is_case_or_bitcase or (p.is_case_or_bitcase and p.has_name):
                 prefix.append((p.name[-1], '.', p))
             fields.update(_c_helper_field_mapping(p, prefix, flat=True))
 
         # auxiliary object for 'S' parameter
         S_obj = parents[1]
 
+    # for functions generated below:
+    # * compute list of additional parameters which contains as parameter
+    #   any expr fields that cannot be resolved within self and descendants.
+    # * and make sure that they are accessed without prefix within the function.
+    unresolved_fields = resolve_expr_fields_list(list, parents)
+    additional_params = []
+    additional_param_names = set();
+    for f in unresolved_fields:
+        if f.c_field_name not in additional_param_names:
+            # add to the list of additional params
+            additional_params.append((f.c_field_type, f.c_field_name));
+            # make sure that the param is accessed without prefix within the function
+            fields[ f.c_field_name ] = (f.c_field_name, f)
+
+    #internal function to compute the parameterlist with given indentation
+    #such that the formatting of the additional parameters is consistent with
+    #the other parameters.
+    def additional_params_to_str(indent):
+        if len(additional_params) == 0:
+            return ''
+        else:
+            return (',\n' + indent).join([''] + ['%s %s /**< */' % p for p in additional_params])
+
     _h_setlevel(1)
     _c_setlevel(1)
     if list.member.fixed_size():
         idx = 1 if switch_obj is not None else 0
         _hc('')
         _hc('%s *', field.c_field_type)
 
@@ -1856,38 +1936,40 @@ def _c_accessors_list(self, field):
                 _c_iterator_get_end(prev_varsized_field, 'R'))
             _c('    return (%s *) ((char *) prev.data + %s + %d);',
                field.c_field_type, align_pad, field.prev_varsized_offset)
         _c('}')
 
     _hc('')
     _hc('int')
+    spacing = ' '*(len(field.c_length_name)+2)
+    add_param_str = additional_params_to_str(spacing)
     if switch_obj is not None:
         _hc('%s (const %s *R  /**< */,', field.c_length_name, R_obj.c_type)
-        spacing = ' '*(len(field.c_length_name)+2)
-        _h('%sconst %s *S /**< */);', spacing, S_obj.c_type)
-        _c('%sconst %s *S  /**< */)', spacing, S_obj.c_type)
+        _h('%sconst %s *S /**< */%s);', spacing, S_obj.c_type, add_param_str)
+        _c('%sconst %s *S  /**< */%s)', spacing, S_obj.c_type, add_param_str)
     else:
-        _h('%s (const %s *R  /**< */);', field.c_length_name, c_type)
-        _c('%s (const %s *R  /**< */)', field.c_length_name, c_type)
+        _h('%s (const %s *R  /**< */%s);', field.c_length_name, c_type, add_param_str)
+        _c('%s (const %s *R  /**< */%s)', field.c_length_name, c_type, add_param_str)
     _c('{')
     length = _c_accessor_get_expr(field.type.expr, fields)
     _c('    return %s;', length)
     _c('}')
 
     if field.type.member.is_simple:
         _hc('')
         _hc('xcb_generic_iterator_t')
+        spacing = ' '*(len(field.c_end_name)+2)
+        add_param_str = additional_params_to_str(spacing)
         if switch_obj is not None:
             _hc('%s (const %s *R  /**< */,', field.c_end_name, R_obj.c_type)
-            spacing = ' '*(len(field.c_end_name)+2)
-            _h('%sconst %s *S /**< */);', spacing, S_obj.c_type)
-            _c('%sconst %s *S  /**< */)', spacing, S_obj.c_type)
+            _h('%sconst %s *S /**< */%s);', spacing, S_obj.c_type, add_param_str)
+            _c('%sconst %s *S  /**< */%s)', spacing, S_obj.c_type, add_param_str)
         else:
-            _h('%s (const %s *R  /**< */);', field.c_end_name, c_type)
-            _c('%s (const %s *R  /**< */)', field.c_end_name, c_type)
+            _h('%s (const %s *R  /**< */%s);', field.c_end_name, c_type, add_param_str)
+            _c('%s (const %s *R  /**< */%s)', field.c_end_name, c_type, add_param_str)
         _c('{')
         _c('    xcb_generic_iterator_t i;')
 
         param = 'R' if switch_obj is None else 'S'
         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))
@@ -1904,22 +1986,23 @@ def _c_accessors_list(self, field):
         _c('    i.index = (char *) i.data - (char *) %s;', param)
         _c('    return i;')
         _c('}')
 
     else:
         _hc('')
         _hc('%s', field.c_iterator_type)
+        spacing = ' '*(len(field.c_iterator_name)+2)
+        add_param_str = additional_params_to_str(spacing)
         if switch_obj is not None:
             _hc('%s (const %s *R  /**< */,', field.c_iterator_name, R_obj.c_type)
-            spacing = ' '*(len(field.c_iterator_name)+2)
-            _h('%sconst %s *S /**< */);', spacing, S_obj.c_type)
-            _c('%sconst %s *S  /**< */)', spacing, S_obj.c_type)
+            _h('%sconst %s *S /**< */%s);', spacing, S_obj.c_type, add_param_str)
+            _c('%sconst %s *S  /**< */%s)', spacing, S_obj.c_type, add_param_str)
         else:
-            _h('%s (const %s *R  /**< */);', field.c_iterator_name, c_type)
-            _c('%s (const %s *R  /**< */)', field.c_iterator_name, c_type)
+            _h('%s (const %s *R  /**< */%s);', field.c_iterator_name, c_type, add_param_str)
+            _c('%s (const %s *R  /**< */%s)', field.c_iterator_name, c_type, add_param_str)
         _c('{')
         _c('    %s i;', field.c_iterator_type)
 
         _c_pre.start()
         length_expr_str = _c_accessor_get_expr(field.type.expr, fields)
 
         if switch_obj is not None:
@@ -1941,14 +2024,21 @@ def _c_accessors_list(self, field):
             _c_pre.end()
             _c('    i.data = (%s *) ((char *) prev.data + %s);',
                 field.c_field_type, align_pad)
 
         if switch_obj is None:
             _c('    i.rem = %s;', length_expr_str )
         _c('    i.index = (char *) i.data - (char *) %s;', 'R' if switch_obj is None else 'S' )
+
+        # initialize additional iterator fields
+        # which are derived from additional type parameters for the list member type.
+        additional_iter_fields = _c_get_additional_type_params(field.type.member)
+        for iter_field in additional_iter_fields:
+             _c('    i.%s = %s;', iter_field[2], fields[iter_field[2]][0])
+
         _c('    return i;')
         _c('}')
 
 def _c_accessors(self, name, base):
     '''
     Declares the accessor functions for the fields of a structure.
     '''
-- 2.0.1




More information about the Xcb mailing list