[Xcb] [PATCH libxcb 1/1] Calculate length of length less lists

Christian Linhart chris at DemoRecorder.com
Thu Jun 11 10:29:38 PDT 2015


Hi Jaya,

This patch does not apply to the current HEAD of the official repo.
Probably there were incompatibilities created by other patches.

Can you please adapt your patch accordingly?
(and please do the changes proposed by Ran in his pre-review)

We probably need this patch for the upcoming release to get
good accessor function-parameterlists for "xcb_present_redirect_notify...".

With "good" I especially mean parameterlists that we will not want to change later.
After a release, parameterlists are cut in stone for ABI/API compatibility reasons.

Thank you,

Chris

P.S.:
When trying to apply it with "patch", the errors are as follows:
Hunk #1 succeeded at 646 (offset -1 lines).
Hunk #2 succeeded at 753 (offset -1 lines).
Hunk #3 succeeded at 770 (offset -1 lines).
Hunk #4 succeeded at 795 (offset -1 lines).
Hunk #5 succeeded at 985 (offset -1 lines).
Hunk #6 succeeded at 1724 (offset -1 lines).
Hunk #7 succeeded at 1805 (offset -1 lines).
Hunk #8 FAILED at 1907.
Hunk #9 FAILED at 1957.
Hunk #10 succeeded at 2158 (offset -1 lines).
Hunk #11 succeeded at 2349 (offset -1 lines).


On 04/23/15 08:47, Jaya Tiwari wrote:
> Signed-off-by: Jaya Tiwari <tiwari.jaya18 at gmail.com>
> ---
>  src/c_client.py | 54 +++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 41 insertions(+), 13 deletions(-)
>
> diff --git a/src/c_client.py b/src/c_client.py
> index e55fc3c..2e3129c 100644
> --- a/src/c_client.py
> +++ b/src/c_client.py
> @@ -647,7 +647,7 @@ def get_expr_fields(self):
>      get the Fields referenced by switch or list expression
>      """
>      def get_expr_field_names(expr):
> -        if expr.op is None:
> +        if expr.op is None or expr.op == 'calculate_len':
>              if expr.lenfield_name is not None:
>                  return [expr.lenfield_name]
>              else:
> @@ -754,6 +754,7 @@ def get_serialize_params(context, self, buffer_var='_buffer', aux_var='_aux'):
>      param_fields = []
>      wire_fields = []
>  
> +    dont_include_len = False
>      for field in self.fields:
>          if field.visible:
>              # the field should appear as a parameter in the function call
> @@ -770,6 +771,8 @@ def get_serialize_params(context, self, buffer_var='_buffer', aux_var='_aux'):
>      if self.is_switch:
>          param_fields = get_expr_fields(self)
>  
> +    if self.is_list and self.type.expr.op == 'calculate_len':
> +       dont_include_len = True
>      # _serialize()/_unserialize()/_unpack() function parameters
>      # note: don't use set() for params, it is unsorted
>      params = []
> @@ -793,7 +796,7 @@ def get_serialize_params(context, self, buffer_var='_buffer', aux_var='_aux'):
>              pointerspec = p.c_pointer
>              add_param(params, (typespec, pointerspec, p.c_field_name))
>          else:
> -            if p.visible and not p.wire and not p.auto:
> +            if p.visible and not p.wire and not p.auto and not dont_include_len:
>                  typespec = p.c_field_type
>                  pointerspec = ''
>                  add_param(params, (typespec, pointerspec, p.c_field_name))
> @@ -983,7 +986,10 @@ def _c_serialize_helper_list_field(context, self, field,
>          if len(unresolved)>0:
>              raise Exception('could not resolve the length fields required for list %s' % field.c_field_name)
>  
> -    list_length = _c_accessor_get_expr(expr, field_mapping)
> +    if expr.op == 'calculate_len':
> +        list_length = field.type.expr.lenfield_name
> +    else:
> +        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)
> @@ -1719,7 +1725,7 @@ def _c_accessor_get_expr(expr, field_mapping):
>          return sumvar
>      elif expr.op == 'listelement-ref':
>          return '(*xcb_listelement)'
> -    elif expr.op != None:
> +    elif expr.op != None and expr.op != 'calculate_len':
>          return ('(' + _c_accessor_get_expr(expr.lhs, field_mapping) +
>                  ' ' + expr.op + ' ' +
>                  _c_accessor_get_expr(expr.rhs, field_mapping) + ')')
> @@ -1800,6 +1806,7 @@ def _c_accessors_list(self, field):
>  
>      list = field.type
>      c_type = self.c_type
> +    dont_need_len = False
>  
>      # special case: switch
>      # in case of switch, 2 params have to be supplied to certain accessor functions:
> @@ -1900,16 +1907,25 @@ 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 field.type.is_list and field.type.expr.op == 'calculate_len':
> +        dont_need_len = True
> +    else:
> +        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)
>          _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:
> +    elif not dont_need_len:
>          _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)
> +    else:
> +        _h('%s (const %s *R  /**< */);', field.c_length_name, c_type)
> +        _c('%s (const %s *R  /**< */)', field.c_length_name, c_type)
>      _c('{')
> -    length = _c_accessor_get_expr(field.type.expr, fields)
> +    if field.type.expr.op == 'calculate_len':
> +        length = '(((R->length * 4) - sizeof('+ self.c_type + '))/'+'sizeof('+field.type.member.c_wiretype+'))'
> +    else:
> +        length = _c_accessor_get_expr(field.type.expr, fields)
>      _c('    return %s;', length)
>      _c('}')
>  
> @@ -1950,19 +1966,26 @@ def _c_accessors_list(self, field):
>          _hc('')
>          _hc('%s', field.c_iterator_type)
>          spacing = ' '*(len(field.c_iterator_name)+2)
> -        add_param_str = additional_params_to_str(spacing)
> +        if field.type.expr.op != 'calculate_len':
> +            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)
>              _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:
> +        elif not dont_need_len:
>              _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)
> +        else:
> +            _h('%s (const %s *R  /**< */);', field.c_iterator_name, c_type)
> +            _c('%s (const %s *R  /**< */)', field.c_iterator_name, c_type)
>          _c('{')
>          _c('    %s i;', field.c_iterator_type)
>  
>          _c_pre.start()
> -        length_expr_str = _c_accessor_get_expr(field.type.expr, fields)
> +        if field.type.expr.op == 'calculate_len':
> +            length_expr_str = '(((R->length * 4) - sizeof('+ self.c_type + '))/'+'sizeof('+field.c_field_type+'))'
> +        else:
> +            length_expr_str = _c_accessor_get_expr(field.type.expr, fields)
>  
>          if switch_obj is not None:
>              _c_pre.end()
> @@ -2152,6 +2175,7 @@ def _c_request_helper(self, name, void, regular, aux=False, reply_fds=False):
>      serial_fields = []
>      # special case: list with variable size elements
>      list_with_var_size_elems = False
> +    have_field_len = False
>  
>      for field in self.fields:
>          if field.visible:
> @@ -2342,9 +2366,13 @@ def _c_request_helper(self, name, void, regular, aux=False, reply_fds=False):
>                      _c('    xcb_parts[%d].iov_base = (char *) %s;', count, field.c_field_name)
>                      if field.type.is_list:
>                          if field.type.member.fixed_size():
> -                            _c('    xcb_parts[%d].iov_len = %s * sizeof(%s);', count,
> -                               _c_accessor_get_expr(field.type.expr, None),
> -                               field.type.member.c_wiretype)
> +                            if field.type.expr.op == 'calculate_len':
> +                                lenfield = field.type.expr.lenfield_name
> +                            else:
> +                                lenfield = _c_accessor_get_expr(field.type.expr, None)
> +
> +                            _c('    xcb_parts[%d].iov_len = %s * sizeof(%s);', count, lenfield,
> +                                        field.type.member.c_wiretype)
>                          else:
>                              list_length = _c_accessor_get_expr(field.type.expr, None)
>  



More information about the Xcb mailing list