From 265d38882cffce597367cc8bb2160b9e2482a80f Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Sun, 2 Nov 2014 13:45:12 +0100 Subject: generator: fix absname for fields with only accessor function Fix _c_helper_absolute_name for fields which cannot be accessed as a struct/union member but which can be accessed by an accessor function. The fix generates calls to the accessor function in these cases. Example: len The sumof-expression ( ) refers to mylist1 which is only acessible by an accessor function. Previously, sumof was only used inside bitcases, where such lists are accessible by members of the deserialized parent struct. (there is a difference between deserialization of switches and structs.) V2 of this patch: * replaced "!= None" with "is not None" because that's more pythonic. (according to suggestion from Ran Benita) V3 of this patch: simplification: * fixed the recursion in _c_type_setup so that _c_helper_absolute_name does not need check a gazillion things as a workaround anymore. * simplified _c_helper_absolute_name - remove unneeded check for empty string before append of last_sep to prefix_str - removed those if-conditions which are not needed anymore after fixing the recursion in _c_type_setup. - extract functionality for checking whether a field needs an accessor ( and which type of accessor ) in functions. (also extracted from _c_accessors) - rearrange the condition branches and actions for more readability. V3 generates exactly the same *.c and *.h files as V2. V4 of this patch: * improve formatting as per suggestions of Ran Signed-off-by: Christian Linhart Reviewed-by: Ran Benita Message-ID: <54562758.5090107@DemoRecorder.com> Patch-Thread-Subject: [Xcb] [PATCHSET] ListInputDevices revision 2 Patch-Set: ListInputDevices Patch-Number: libxcb 1/9 Patch-Version: V4 --- src/c_client.py | 64 ++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index 56a1766..ac5b6dc 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -336,12 +336,6 @@ def _c_type_setup(self, name, postfix): first_field_after_varsized = None for field in self.fields: - _c_type_setup(field.type, field.field_type, ()) - if field.type.is_list: - _c_type_setup(field.type.member, field.field_type, ()) - if (field.type.nmemb is None): - self.c_need_sizeof = True - field.c_field_type = _t(field.field_type) field.c_field_const_type = ('' if field.type.nmemb == 1 else 'const ') + field.c_field_type field.c_field_name = _cpp(field.field_name) @@ -391,6 +385,15 @@ def _c_type_setup(self, name, postfix): if field.type.fixed_size(): field.prev_varsized_field = None + # recurse into this field this has to be done here, i.e., + # after the field has been set up. Otherwise the function + # _c_helper_absolute_name will produce garbage or crash + _c_type_setup(field.type, field.field_type, ()) + if field.type.is_list: + _c_type_setup(field.type.member, field.field_type, ()) + if (field.type.nmemb is None): + self.c_need_sizeof = True + if self.c_need_serialize: # when _unserialize() is wanted, create _sizeof() as well for consistency reasons self.c_need_sizeof = True @@ -427,6 +430,24 @@ def _c_type_setup(self, name, postfix): _c_serialize('sizeof', self) # _c_type_setup() +# Functions for querying field properties +def _c_field_needs_list_accessor(field): + return field.type.is_list and not field.type.fixed_size() + +def _c_field_needs_field_accessor(field): + if field.type.is_list: + return False + else: + return (field.prev_varsized_field is not None or + not field.type.fixed_size()) + +def _c_field_needs_accessor(field): + return (_c_field_needs_list_accessor(field) or + _c_field_needs_field_accessor(field)) + +def _c_field_is_member_of_case_or_bitcase(field): + return field.parent and field.parent.is_case_or_bitcase + def _c_helper_absolute_name(prefix, field=None): """ turn prefix, which is a list of tuples (name, separator, Type obj) into a string @@ -434,16 +455,35 @@ def _c_helper_absolute_name(prefix, field=None): if field is not None, append the field name as well """ prefix_str = '' + last_sep ='' for name, sep, obj in prefix: - prefix_str += name + prefix_str += last_sep + name if '' == sep: sep = '->' if ((obj.is_case_or_bitcase and obj.has_name) or # named bitcase (obj.is_switch and len(obj.parents)>1)): sep = '.' - prefix_str += sep - if field is not None: - prefix_str += _cpp(field.field_name) + last_sep = sep + + if field is None: + # add separator for access to a yet unknown field + prefix_str += last_sep + else: + if _c_field_needs_accessor(field): + if _c_field_is_member_of_case_or_bitcase(field): + # case members are available in the deserialized struct, + # so there is no need to use the accessor function + # (also, their accessor function needs a different arglist + # so this would require special treatment here) + # Therefore: Access as struct member + prefix_str += last_sep + _cpp(field.field_name) + else: + # Access with the accessor function + prefix_str = field.c_accessor_name + "(" + prefix_str + ")" + else: + # Access as struct member + prefix_str += last_sep + _cpp(field.field_name) + return prefix_str # _c_absolute_name @@ -1734,9 +1774,9 @@ def _c_accessors(self, name, base): if True: for field in self.fields: if not field.type.is_pad: - if field.type.is_list and not field.type.fixed_size(): + if _c_field_needs_list_accessor(field): _c_accessors_list(self, field) - elif field.prev_varsized_field is not None or not field.type.fixed_size(): + elif _c_field_needs_field_accessor(field): _c_accessors_field(self, field) def c_simple(self, name): -- cgit v1.2.3 From fda1fb4ed47a705744677a0074d83464af7aa4eb Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Sun, 2 Nov 2014 13:45:29 +0100 Subject: generator: expressions can generate pre-code This patch provides a mechanism for generating preparatory code for expressions. This is e.g. necessary when an expression needs computations which cannot be done in a C-Expression, like for-loops. This will be used for sumof expressions but may be useful elsewhere. Note: Patch 2 of this series has been removed during the review process. V2: adapt to changes in previous patches V3: some style and formatting changes according to suggestions from Ran Benita. Signed-off-by: Christian Linhart Reviewed-by: Ran Benita Message-ID: <54562769.3090405@DemoRecorder.com> Patch-Thread-Subject: [Xcb] [PATCHSET] ListInputDevices revision 2 Patch-Set: ListInputDevices Patch-Number: libxcb 3/9 Patch-Version: V3 --- src/c_client.py | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 120 insertions(+), 4 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index ac5b6dc..789b49e 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -58,6 +58,107 @@ def _hc(fmt, *args): _h(fmt, *args) _c(fmt, *args) +def _c_wr_stringlist(indent, strlist): + ''' + Writes the given list of strings to the source file. + Each line is prepended by the indent string + ''' + for str in strlist: + _c("%s%s", indent, str) + + +class PreCode(object): + ''' + For pre-code generated by expression generation + (for example, the for-loop of a sumof) + This has to account for recursiveness of the expression + generation, i.e., there may be pre-code for pre-code. + Therefore this is implemented as a stack of lists of lines. + + If redirection is switched on, then all output is collected in + self.redirect_code and self.redirect_tempvars instead of + being sent to the output via _h und _c. + ''' + def __init__(self): + self.nesting_level = 0 + self.tempvars = [] + self.codelines = [] + self.redirect_code = None + self.redirect_tempvars = None + self.indent_str = ' ' + self.indent_stack = [] + self.tempvar_num = 0 + + + # start and end of pre-code blocks + def start(self): + self.nesting_level += 1 + + def end(self): + self.nesting_level -= 1 + if self.nesting_level == 0: + # lowest pre-code level is finished -> output to source + if self.redirect_tempvars is None: + _c_wr_stringlist('', self.tempvars) + self.tempvars = [] + else: + self.redirect_tempvars.extend(self.tempvars) + self.tempvars = [] + if self.redirect_code == None: + _c_wr_stringlist('', self.codelines) + self.codelines = [] + else: + self.redirect_code.extend(self.codelines) + self.codelines = [] + + + def output_tempvars(self): + if self.redirect_code == None: + _c_wr_stringlist('', self.tempvars) + self.tempvars = [] + + # output to precode + def code(self, fmt, *args): + self.codelines.append(self.indent_str + fmt % args) + + def tempvar(self, fmt, *args): + self.tempvars.append(' ' + (fmt % args)) + + # get a unique name for a temporary variable + def get_tempvarname(self): + self.tempvar_num += 1 + return "xcb_pre_tmp_%d" % self.tempvar_num + + # indentation + + def push_indent(self, indentstr): + self.indent_stack.append(self.indent_str) + self.indent_str = indentstr + + def push_addindent(self, indent_add_str): + self.push_indent(self.indent_str + indent_add_str) + + def indent(self): + self.push_addindent(' ') + + def pop_indent(self): + self.indent_str = self.indent_stack.pop() + + # redirection to lists + def redirect_start(self, redirect_code, redirect_tempvars=None): + self.redirect_code = redirect_code + self.redirect_tempvars = redirect_tempvars + if redirect_tempvars is not None: + self.tempvar_num = 0 + + def redirect_end(self): + self.redirect_code = None + self.redirect_tempvars = None + +# global PreCode handler +_c_pre = PreCode() + + # XXX See if this level thing is really necessary. def _h_setlevel(idx): ''' @@ -1024,6 +1125,8 @@ def _c_serialize_helper_fields(context, self, need_padding = False prev_field_was_variable = False + _c_pre.push_indent(space + ' ') + for field in self.fields: if not field.visible: if not ((field.wire and not field.auto) or 'unserialize' == context): @@ -1112,6 +1215,8 @@ def _c_serialize_helper_fields(context, self, if self.c_var_followed_by_fixed_fields: need_padding = False + _c_pre.pop_indent() + return count # _c_serialize_helper_fields() @@ -1210,6 +1315,8 @@ def _c_serialize(context, self): temp_vars = [] prefix = [] + _c_pre.redirect_start(code_lines, temp_vars) + if 'serialize' == context: if not self.is_switch and not self.c_var_followed_by_fixed_fields: _c(' %s *xcb_out = *_buffer;', self.c_type) @@ -1255,11 +1362,13 @@ def _c_serialize(context, self): _c(' %s _aux;', self.c_type) _c(' return %s(%s, &_aux);', self.c_unpack_name, reduce(lambda x,y: "%s, %s" % (x, y), param_names)) _c('}') + _c_pre.redirect_end() return elif self.c_var_followed_by_fixed_fields: # special case: call _unserialize() _c(' return %s(%s, NULL);', self.c_unserialize_name, reduce(lambda x,y: "%s, %s" % (x, y), param_names)) _c('}') + _c_pre.redirect_end() return else: _c(' char *xcb_tmp = (char *)_buffer;') @@ -1295,6 +1404,8 @@ def _c_serialize(context, self): _c(' unsigned int xcb_pad = 0;') _c(' unsigned int xcb_align_to = 0;') + _c_pre.redirect_end() + _c('') for t in temp_vars: _c(t) @@ -1684,12 +1795,11 @@ def _c_accessors_list(self, field): 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) - length = _c_accessor_get_expr(field.type.expr, fields) else: _h('%s (const %s *R /**< */);', field.c_length_name, c_type) _c('%s (const %s *R /**< */)', field.c_length_name, c_type) - length = _c_accessor_get_expr(field.type.expr, fields) _c('{') + length = _c_accessor_get_expr(field.type.expr, fields) _c(' return %s;', length) _c('}') @@ -1739,10 +1849,15 @@ def _c_accessors_list(self, field): _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: + _c_pre.end() _c(' i.data = %s;', fields[field.c_field_name][0]) - _c(' i.rem = %s;', _c_accessor_get_expr(field.type.expr, fields)) + _c(' i.rem = %s;', length_expr_str) elif field.prev_varsized_field == None: + _c_pre.end() _c(' i.data = (%s *) (R + 1);', field.c_field_type) else: (prev_varsized_field, align_pad) = get_align_pad(field) @@ -1753,11 +1868,12 @@ def _c_accessors_list(self, field): _c(' xcb_generic_iterator_t prev = %s;', _c_iterator_get_end(prev_varsized_field, 'R')) + _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;', _c_accessor_get_expr(field.type.expr, fields)) + _c(' i.rem = %s;', length_expr_str) _c(' i.index = (char *) i.data - (char *) %s;', 'R' if switch_obj is None else 'S' ) _c(' return i;') _c('}') -- cgit v1.2.3 From 4a915c0dbadf326ea61349e29a0029d29f4bd339 Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Sun, 2 Nov 2014 13:45:40 +0100 Subject: generator: sumof: support any type, generate explicit code A sumof-expression now generates explicit code ( for-loop etc ) instead of calling xcb_sumof. This way, it supports any type which can be added. Previously, only uint_8 was supported. Here's an example and the generated code: xml: len declaration of tempvars at the start of enclosing function: int xcb_pre_tmp_1; /* sumof length */ int xcb_pre_tmp_2; /* sumof loop counter */ int64_t xcb_pre_tmp_3; /* sumof sum */ const uint16_t* xcb_pre_tmp_4; /* sumof list ptr */ code: /* mylist2 */ /* sumof start */ xcb_pre_tmp_1 = _aux->len; xcb_pre_tmp_3 = 0; xcb_pre_tmp_4 = xcb_input_sumof_test_mylist_1(_aux); for ( xcb_pre_tmp_2 = 0; xcb_pre_tmp_2 < xcb_pre_tmp_1; xcb_pre_tmp_2++) { xcb_pre_tmp_3 += *xcb_pre_tmp_4; xcb_pre_tmp_4++; } /* sumof end. Result is in xcb_pre_tmp_3 */ xcb_block_len += xcb_pre_tmp_3 * sizeof(uint8_t); This patch is also a preparation for sumof which can access fields of lists of struct, etc. V2: Changed style and formatting according to suggestions from Ran Benita Signed-off-by: Christian Linhart Reviewed-by: Ran Benita Message-ID: <54562774.8030306@DemoRecorder.com> Patch-Thread-Subject: [Xcb] [PATCHSET] ListInputDevices revision 2 Patch-Set: ListInputDevices Patch-Number: libxcb 4/9 Patch-Version: V2 --- src/c_client.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index 789b49e..0fae837 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -1628,9 +1628,31 @@ def _c_accessor_get_expr(expr, field_mapping): raise Exception("list field '%s' referenced by sumof not found" % expr.lenfield_name) list_name = field_mapping[field.c_field_name][0] c_length_func = "%s(%s)" % (field.c_length_name, list_name) - # note: xcb_sumof() has only been defined for integers c_length_func = _c_accessor_get_expr(field.type.expr, field_mapping) - return 'xcb_sumof(%s, %s)' % (list_name, c_length_func) + # create explicit code for computing the sum. + # This works for all C-types which can be added to int64_t with += + _c_pre.start() + lengthvar = _c_pre.get_tempvarname() + loopvar = _c_pre.get_tempvarname() + sumvar = _c_pre.get_tempvarname() + listvar = _c_pre.get_tempvarname() + _c_pre.tempvar("int %s; /* sumof length */", lengthvar) + _c_pre.tempvar("int %s; /* sumof loop counter */", loopvar) + _c_pre.tempvar("int64_t %s; /* sumof sum */", sumvar) + _c_pre.tempvar("const %s* %s; /* sumof list ptr */", field.c_field_type, listvar) + _c_pre.code("/* sumof start */") + _c_pre.code("%s = %s;", lengthvar, c_length_func) + _c_pre.code("%s = 0;", sumvar) + _c_pre.code("%s = %s;", listvar, list_name) + _c_pre.code("for (%s = 0; %s < %s; %s++) {", loopvar, loopvar, lengthvar, loopvar) + _c_pre.indent() + _c_pre.code("%s += *%s;", sumvar, listvar) + _c_pre.code("%s++;", listvar) + _c_pre.pop_indent() + _c_pre.code("}") + _c_pre.code("/* sumof end. Result is in %s */", sumvar) + _c_pre.end() + return sumvar elif expr.op != None: return ('(' + _c_accessor_get_expr(expr.lhs, field_mapping) + ' ' + expr.op + ' ' + -- cgit v1.2.3 From 51a0d57acc6ec0c9487d2dbc2dda806f05c49884 Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Sun, 2 Nov 2014 13:46:16 +0100 Subject: generator: sumof with nested expression Support sumof with a nested expression. The nested expression is computed for every list-element and the result of the computation is added to the sum. This way, sumof can be applied to a list of structs, and, e.g., compute the sum of a specific field of that struct. example: len bar generated tmpvar: int xcb_pre_tmp_1; /* sumof length */ int xcb_pre_tmp_2; /* sumof loop counter */ int64_t xcb_pre_tmp_3; /* sumof sum */ const xcb_input_sumof_test_element_t* xcb_pre_tmp_4; /* sumof list ptr */ generated code: /* mylist2 */ /* sumof start */ xcb_pre_tmp_1 = _aux->len; xcb_pre_tmp_3 = 0; xcb_pre_tmp_4 = xcb_input_sumof_test_field_access_mylist_1(_aux); for ( xcb_pre_tmp_2 = 0; xcb_pre_tmp_2 < xcb_pre_tmp_1; xcb_pre_tmp_2++) { xcb_pre_tmp_3 += xcb_pre_tmp_4->bar; xcb_pre_tmp_4++; } /* sumof end. Result is in xcb_pre_tmp_3 */ xcb_block_len += xcb_pre_tmp_3 * sizeof(uint16_t); changes for V2 of this patch: * explicitely set the member access operator in the prefix-tuple passed to function _c_helper_field_mapping. This enables us to simplify function "_c_helper_absolute_name" (which will be renamed "_c_helper_fieldaccess_expr" soon) V3: Changed style and formatting according to suggestions from Ran Benita Signed-off-by: Christian Linhart Reviewed-by: Ran Benita Message-ID: <54562798.8040500@DemoRecorder.com> Patch-Thread-Subject: [Xcb] [PATCHSET] ListInputDevices revision 2 Patch-Set: ListInputDevices Patch-Number: libxcb 5/9 Patch-Version: V3 --- src/c_client.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/c_client.py b/src/c_client.py index 0fae837..eae1f6c 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -1646,7 +1646,30 @@ def _c_accessor_get_expr(expr, field_mapping): _c_pre.code("%s = %s;", listvar, list_name) _c_pre.code("for (%s = 0; %s < %s; %s++) {", loopvar, loopvar, lengthvar, loopvar) _c_pre.indent() - _c_pre.code("%s += *%s;", sumvar, listvar) + + if expr.rhs is None: + _c_pre.code("%s += *%s;", sumvar, listvar) + else: + # sumof has a nested expression which has to be evaluated in + # the context of this list element + + # field mapping for the subexpression needs to include + # the fields of the list-member type + scoped_field_mapping = field_mapping.copy() + scoped_field_mapping.update( + _c_helper_field_mapping( + field.type.member, + [(listvar, '->', field.type.member)])) + + # cause pre-code of the subexpression be added right here + _c_pre.end() + # compute the subexpression + rhs_expr_str = _c_accessor_get_expr(expr.rhs, scoped_field_mapping) + # resume with our code + _c_pre.start() + # output the summation expression + _c_pre.code("%s += %s;", sumvar, rhs_expr_str) + _c_pre.code("%s++;", listvar) _c_pre.pop_indent() _c_pre.code("}") -- cgit v1.2.3 From 17f6e04493c93014beaf3a704c8cc4d050cb13bf Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Sun, 2 Nov 2014 13:46:38 +0100 Subject: rename _c_helper_absolute_name to _c_helper_fieldaccess_expr The function _c_helper_absolute_name was named in a misleading way. It computes a C-expression for accessing a field of an xcb-type. Therefore the name _c_helper_fieldaccess_expr is more appropriate. Note: Patch 6 of this series has been removed during the review process. Signed-off-by: Christian Linhart Reviewed-by: Ran Benita Message-ID: <545627AE.2040200@DemoRecorder.com> Patch-Thread-Subject: [Xcb] [PATCHSET] ListInputDevices revision 2 Patch-Set: ListInputDevices Patch-Number: libxcb 7/9 Patch-Version: V1 --- src/c_client.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index eae1f6c..409054b 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -488,7 +488,7 @@ def _c_type_setup(self, name, postfix): # recurse into this field this has to be done here, i.e., # after the field has been set up. Otherwise the function - # _c_helper_absolute_name will produce garbage or crash + # _c_helper_fieldaccess_expr will produce garbage or crash _c_type_setup(field.type, field.field_type, ()) if field.type.is_list: _c_type_setup(field.type.member, field.field_type, ()) @@ -549,7 +549,7 @@ def _c_field_needs_accessor(field): def _c_field_is_member_of_case_or_bitcase(field): return field.parent and field.parent.is_case_or_bitcase -def _c_helper_absolute_name(prefix, field=None): +def _c_helper_fieldaccess_expr(prefix, field=None): """ turn prefix, which is a list of tuples (name, separator, Type obj) into a string representing a valid name in C (based on the context) @@ -606,7 +606,7 @@ def _c_helper_field_mapping(complex_type, prefix, flat=False): all_fields.update(_c_helper_field_mapping(b.type, bitcase_prefix, flat)) else: for f in complex_type.fields: - fname = _c_helper_absolute_name(prefix, f) + fname = _c_helper_fieldaccess_expr(prefix, f) if f.field_name in all_fields: raise Exception("field name %s has been registered before" % f.field_name) @@ -891,7 +891,7 @@ def _c_serialize_helper_switch_field(context, self, field, c_switch_variable, pr # switch is handled by this function as a special case param_fields, wire_fields, params = get_serialize_params(context, self) field_mapping = _c_helper_field_mapping(self, prefix) - prefix_str = _c_helper_absolute_name(prefix) + prefix_str = _c_helper_fieldaccess_expr(prefix) # find the parameters that need to be passed to _serialize()/_unpack(): # all switch expr fields must be given as parameters @@ -935,7 +935,7 @@ def _c_serialize_helper_list_field(context, self, field, helper function to cope with lists of variable length """ expr = field.type.expr - prefix_str = _c_helper_absolute_name(prefix) + prefix_str = _c_helper_fieldaccess_expr(prefix) param_fields, wire_fields, params = get_serialize_params('sizeof', self) param_names = [p[2] for p in params] @@ -1006,7 +1006,7 @@ def _c_serialize_helper_fields_fixed_size(context, self, field, typename = reduce(lambda x,y: "%s.%s" % (x, y), scoped_name) code_lines.append('%s /* %s.%s */' % (space, typename, field.c_field_name)) - abs_field_name = _c_helper_absolute_name(prefix, field) + abs_field_name = _c_helper_fieldaccess_expr(prefix, field) # default for simple cases: call sizeof() length = "sizeof(%s)" % field.c_field_type @@ -1068,7 +1068,7 @@ def _c_serialize_helper_fields_fixed_size(context, self, field, def _c_serialize_helper_fields_variable_size(context, self, field, code_lines, temp_vars, space, prefix): - prefix_str = _c_helper_absolute_name(prefix) + prefix_str = _c_helper_fieldaccess_expr(prefix) if context in ('unserialize', 'unpack', 'sizeof'): value = '' -- cgit v1.2.3 From d905b886185fd8e0d9d7f70e0d112cb58ab86f98 Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Sun, 2 Nov 2014 13:46:50 +0100 Subject: function _c_helper_fieldaccess_expr: improve description Signed-off-by: Christian Linhart Reviewed-by: Ran Benita Message-ID: <545627BA.1000909@DemoRecorder.com> Patch-Thread-Subject: [Xcb] [PATCHSET] ListInputDevices revision 2 Patch-Set: ListInputDevices Patch-Number: libxcb 8/9 Patch-Version: V1 --- src/c_client.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index 409054b..5a41f9a 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -552,8 +552,17 @@ def _c_field_is_member_of_case_or_bitcase(field): def _c_helper_fieldaccess_expr(prefix, field=None): """ turn prefix, which is a list of tuples (name, separator, Type obj) into a string - representing a valid name in C (based on the context) - if field is not None, append the field name as well + representing a valid field-access-expression in C (based on the context) + if field is not None, append access to the field as well. + + "separator" is one of the C-operators "." or "->". + + A field access expression can consist of the following components: + * struct/union member access from a value with the "."-operator + * struct/union member access from a pointer with "->"-operator + * function-call of an accessor function: + This is used when a xcb-field is not contained in a struct. + This can, e.g., happen for fields after var-sized fields, etc. """ prefix_str = '' last_sep ='' -- cgit v1.2.3 From 18ff453edd42712ea4d1e7218bbe8829f9f4caba Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Sun, 2 Nov 2014 13:46:58 +0100 Subject: _c_helper_fieldaccess_expr: remove handling for empty sep The loop-variable "sep" is never empty in function "_c_helper_fieldaccess_expr", after a fix elsewhere. Therefore I removed the handling of the case of "sep" being empty. Thanks to Ran Benita for the hint that this can be removed. Signed-off-by: Christian Linhart Reviewed-by: Ran Benita Message-ID: <545627C2.3050608@DemoRecorder.com> Patch-Thread-Subject: [Xcb] [PATCHSET] ListInputDevices revision 2 Patch-Set: ListInputDevices Patch-Number: libxcb 9/9 Patch-Version: V1 --- src/c_client.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index 5a41f9a..21359b5 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -568,11 +568,6 @@ def _c_helper_fieldaccess_expr(prefix, field=None): last_sep ='' for name, sep, obj in prefix: prefix_str += last_sep + name - if '' == sep: - sep = '->' - if ((obj.is_case_or_bitcase and obj.has_name) or # named bitcase - (obj.is_switch and len(obj.parents)>1)): - sep = '.' last_sep = sep if field is None: -- cgit v1.2.3 From 6234225b4be862c5882bf547d066c5a3885906dd Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Thu, 4 Sep 2014 17:50:48 +0200 Subject: generator: no type-setup for eventcopies anymore _c_type_setup is not called for eventcopies anymore: Reasons: * the type-setup of an eventcopy would overwrite members of the original event object such as c_type, ... * it is needed for the next patch, i.e., generating accessors: type_setup would create sizeof-etc funtions which called undefined accessor functions. Sizeof-functions are generated for compatibility: Reason: * Type-setup of eventcopies has previously generated sizeof-functions for eventcopies. So, we still need to generate these functions. These new sizeof-functions simply call the sizeof-function of the defining event of the eventcopy. Message-ID: <1409845851-38950-1-git-send-email-chris@demorecorder.com> Patch-Thread-Subject: [Xcb] support popcount of a list and associated xml changes Patch-Set: PopcountList Patch-Number: libxcb 1/4 Patch-Version: V1 Signed-off-by: Christian Linhart --- src/c_client.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/c_client.py b/src/c_client.py index 21359b5..9c55b58 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -3075,7 +3075,14 @@ def c_event(self, name): force_packed = any(f.type.size == 8 and f.type.is_simple for f in self.fields[(idx+1):]) break - _c_type_setup(self, name, ('event',)) + if self.name == name: + _c_type_setup(self, name, ('event',)) + else: + # no type-setup needed for eventcopies + # (the type-setup of an eventcopy would overwrite members of the original + # event, and it would create sizeof-etc funtions which + # called undefined accessor functions) + pass # Opcode define _c_opcode(name, self.opcodes[name]) @@ -3088,6 +3095,22 @@ def c_event(self, name): _h('') _h('typedef %s %s;', _t(self.name + ('event',)), _t(name + ('event',))) + # Create sizeof-function for eventcopies for compatibility reasons + if self.c_need_sizeof: + _h_setlevel(1) + _c_setlevel(1) + _h('') + _h('int') + _h('%s (const void *_buffer /**< */);', _n(name + ('sizeof',))) + _c('') + _c('int') + _c('%s (const void *_buffer /**< */)', _n(name + ('sizeof',))) + _c('{'); + _c(' return %s(_buffer);', _n(self.name + ('sizeof',))) + _c('}'); + _h_setlevel(0) + _c_setlevel(0) + _man_event(self, name) def c_error(self, name): -- cgit v1.2.3 From b1e4a3bbd8194d12d7fcd9705fcbbe0deb59bcba Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Thu, 4 Sep 2014 17:50:49 +0200 Subject: generator: generate accessors for events, too Accessors are generally needed for var-sized fields and fields after var-sized fields. Generic events can have ver-sized fields. Therefore they need accessors. Message-ID: <1409845851-38950-2-git-send-email-chris@demorecorder.com> Patch-Thread-Subject: [Xcb] support popcount of a list and associated xml changes Patch-Set: PopcountList Patch-Number: libxcb 2/4 Patch-Version: V1 Signed-off-by: Christian Linhart --- src/c_client.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/c_client.py b/src/c_client.py index 9c55b58..9c71e1a 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -3077,6 +3077,10 @@ def c_event(self, name): if self.name == name: _c_type_setup(self, name, ('event',)) + # generate accessors + # (needed for fields after var-sized fields, for lists with var-sized elements, + # switches, ...) + _c_accessors(self, name, name) else: # no type-setup needed for eventcopies # (the type-setup of an eventcopy would overwrite members of the original -- cgit v1.2.3 From 422458b66380e4103c4937f0e2e8bb93e31f273a Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Thu, 4 Sep 2014 17:50:50 +0200 Subject: generator: _c_accessor_get_length: remove buggy special case The function _c_accessor_get_length had a special case handling for intermixed var and fixed size fields. However: * The implementation of that special case was buggy: It tried to call a python-dict as a function which causes Python to abort the program with a stacktrace and error message. So this code was never used. * The case it tried to handle is handeled elsewhere in the meantime: in _c_helper_absolute_name by previous patches made by me. Message-ID: <1409845851-38950-3-git-send-email-chris@demorecorder.com> Patch-Thread-Subject: [Xcb] support popcount of a list and associated xml changes Patch-Set: PopcountList Patch-Number: libxcb 3/4 Patch-Version: V1 Signed-off-by: Christian Linhart --- src/c_client.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index 9c71e1a..88321f4 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -1590,12 +1590,7 @@ def _c_accessor_get_length(expr, field_mapping=None): if field_mapping is not None: lenfield_name = field_mapping[lenfield_name][0] - if expr.lenfield is not None and expr.lenfield.prev_varsized_field is not None: - # special case: variable and fixed size fields are intermixed - # if the lenfield is among the fixed size fields, there is no need - # to call a special accessor function like - return field_mapping(expr.lenfield_name) - elif expr.lenfield_name is not None: + if expr.lenfield_name is not None: return lenfield_name else: return str(expr.nmemb) -- cgit v1.2.3 From 912cd97a6dd019e9e7ecf09c82c4577dd2ad7529 Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Mon, 3 Nov 2014 09:58:08 +0100 Subject: generator: support listelement-ref Support for listelement-ref needs the following three changes (in the order as they appear in the patch): * making the current list-element accessible with the variable xcb_listelement which is a pointer to the list-element * supporting lists of simple-type for sumof with a nested expression * using the variable for resolving a listelement-ref expression Changes for V2 of this patch: - adapt to removal of patch "libxcb 2/6" from patchset "ListInputDevices". Changes for V3 of this patch: - adapt to V2 of patch "libxcb 5/6" from patchset "ListInputDevices" Changes for V4 of this patch: - adapt to revision 2 of the patchset "ListInputDevices" Message-ID: <545743A0.50907@DemoRecorder.com> Patch-Thread-Subject: [Xcb] support popcount of a list and associated xml changes Patch-Set: PopcountList Patch-Number: libxcb 4/4 Patch-Version: V4 Signed-off-by: Christian Linhart --- src/c_client.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index 88321f4..904bfce 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -1646,6 +1646,14 @@ def _c_accessor_get_expr(expr, field_mapping): _c_pre.code("for (%s = 0; %s < %s; %s++) {", loopvar, loopvar, lengthvar, loopvar) _c_pre.indent() + # define and set xcb_listelement, so that it can be used by + # listelement-ref expressions. + if expr.contains_listelement_ref: + _c_pre.code( + "const %s *xcb_listelement = %s;", + field.c_field_type, listvar) + + # summation if expr.rhs is None: _c_pre.code("%s += *%s;", sumvar, listvar) else: @@ -1655,10 +1663,11 @@ def _c_accessor_get_expr(expr, field_mapping): # field mapping for the subexpression needs to include # the fields of the list-member type scoped_field_mapping = field_mapping.copy() - scoped_field_mapping.update( - _c_helper_field_mapping( - field.type.member, - [(listvar, '->', field.type.member)])) + if not field.type.member.is_simple: + scoped_field_mapping.update( + _c_helper_field_mapping( + field.type.member, + [(listvar, '->', field.type.member)])) # cause pre-code of the subexpression be added right here _c_pre.end() @@ -1675,6 +1684,8 @@ def _c_accessor_get_expr(expr, field_mapping): _c_pre.code("/* sumof end. Result is in %s */", sumvar) _c_pre.end() return sumvar + elif expr.op == 'listelement-ref': + return '(*xcb_listelement)' elif expr.op != None: return ('(' + _c_accessor_get_expr(expr.lhs, field_mapping) + ' ' + expr.op + ' ' + -- cgit v1.2.3 From c6f3fb2529a6211221e8254f58c85fd67c1d8844 Mon Sep 17 00:00:00 2001 From: Christian Linhart Date: Mon, 3 Nov 2014 09:57:59 +0100 Subject: generator: support parametrized structs 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 . 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. changes for V2 of this patch: * adapt to revision 2 of patchset ListInputDevices * style fixes for similar things that Ran Benita has found in my previous patches Message-ID: <54574397.4060000@DemoRecorder.com> Patch-Thread-Subject: [Xcb] parametrized structs implemented Patch-Set: ParametrizedStruct Patch-Number: libxcb 1/1 Patch-Version: V3 Signed-off-by: Christian Linhart --- src/c_client.py | 129 +++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 109 insertions(+), 20 deletions(-) diff --git a/src/c_client.py b/src/c_client.py index 904bfce..cce620f 100644 --- a/src/c_client.py +++ b/src/c_client.py @@ -723,6 +723,31 @@ def resolve_expr_fields(complex_obj): 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: @@ -932,6 +957,16 @@ def _c_serialize_helper_switch_field(context, self, field, c_switch_variable, pr return length # _c_serialize_helper_switch_field() +def _c_get_additional_type_params(type): + """ + compute list of additional params for functions created for the given type + """ + if type.is_simple: + return [] + else: + param_fields, wire_fields, params = get_serialize_params('sizeof', type) + return params[1:] + def _c_serialize_helper_list_field(context, self, field, code_lines, temp_vars, space, prefix): @@ -970,6 +1005,14 @@ def _c_serialize_helper_list_field(context, self, field, # 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;' @@ -981,8 +1024,8 @@ def _c_serialize_helper_list_field(context, self, field, # 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) @@ -991,7 +1034,7 @@ def _c_serialize_helper_list_field(context, self, field, 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) @@ -1502,6 +1545,14 @@ def _c_iterator(self, name): _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) @@ -1529,9 +1580,14 @@ def _c_iterator(self, name): _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')) @@ -1815,6 +1871,29 @@ def _c_accessors_list(self, field): # 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(): @@ -1845,14 +1924,15 @@ def _c_accessors_list(self, field): _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) @@ -1861,14 +1941,15 @@ def _c_accessors_list(self, field): 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;') @@ -1893,14 +1974,15 @@ def _c_accessors_list(self, field): 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) @@ -1930,6 +2012,13 @@ def _c_accessors_list(self, field): 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('}') -- cgit v1.2.3