[Xcb] [PATCH libxcb 4/4 V3] generator: fix align-pads for switches which start at unaligned pos
Christian Linhart
chris at DemoRecorder.com
Wed Sep 10 23:10:26 PDT 2014
Hi Ran,
On 09/10/14 19:10, Ran Benita wrote:
> On Tue, Sep 09, 2014 at 11:26:40PM +0200, Christian Linhart wrote:
>> V3: patch revised according to suggestion by Ran Benita:
>> * only create and use xcb_padding_offset for switch
>
> It's always unfortunate when c_client.py becomes even more obscure..
Yes, I agree.
This could be remedied by refactoring the generator using OO-design,
e.g.,. replacing nested structures of tuples and lists by classes, etc.
But I fear that none of us has the time (and/or budget) to do that.
On the other hand, I think that the generator will not need many more
changes after we'll be done with all the changes which are required for
supporting XInput and XKB. I guess that the protocol structure of the
other extensions is simpler than that of XInput and XKB. ( of course
there'll be a some exceptions... )
So the complexity of the generator may remain manageable with its current design.
> But
> this looks good to me now:
>
> Reviewed-by: Ran Benita <ran234 at gmail.com>
Thank you for your review and the other reviews.
I appreciate the effort that you invest in this,
as well as your feedback and your requests to raise the quality of the changes.
Chris
>
>> ---
>> src/c_client.py | 26 +++++++++++++++++++++-----
>> 1 file changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/c_client.py b/src/c_client.py
>> index 1d1bbf6..0a1f877 100644
>> --- a/src/c_client.py
>> +++ b/src/c_client.py
>> @@ -653,17 +653,23 @@ def get_serialize_params(context, self, buffer_var='_buffer', aux_var='_aux'):
>> for p in param_fields:
>> if not p.type.fixed_size():
>> add_param(params, (p.c_field_const_type, '*', p.c_field_name))
>>
>> return (param_fields, wire_fields, params)
>> # get_serialize_params()
>>
>> -def _c_serialize_helper_insert_padding(context, code_lines, space, postpone):
>> +def _c_serialize_helper_insert_padding(context, code_lines, space, postpone, is_case_or_bitcase):
>> code_lines.append('%s /* insert padding */' % space)
>> - code_lines.append('%s xcb_pad = -xcb_block_len & (xcb_align_to - 1);' % space)
>> + if is_case_or_bitcase:
>> + code_lines.append(
>> + '%s xcb_pad = -(xcb_block_len + xcb_padding_offset) & (xcb_align_to - 1);'
>> + % space)
>> + else:
>> + code_lines.append(
>> + '%s xcb_pad = -xcb_block_len & (xcb_align_to - 1);' % space)
>> # code_lines.append('%s printf("automatically inserting padding: %%%%d\\n", xcb_pad);' % space)
>> code_lines.append('%s xcb_buffer_len += xcb_block_len + xcb_pad;' % space)
>>
>> if not postpone:
>> code_lines.append('%s if (0 != xcb_pad) {' % space)
>>
>> if 'serialize' == context:
>> @@ -673,14 +679,16 @@ def _c_serialize_helper_insert_padding(context, code_lines, space, postpone):
>> elif context in ('unserialize', 'unpack', 'sizeof'):
>> code_lines.append('%s xcb_tmp += xcb_pad;' % space)
>>
>> code_lines.append('%s xcb_pad = 0;' % space)
>> code_lines.append('%s }' % space)
>>
>> code_lines.append('%s xcb_block_len = 0;' % space)
>> + if is_case_or_bitcase:
>> + code_lines.append('%s xcb_padding_offset = 0;' % space)
>>
>> # keep tracking of xcb_parts entries for serialize
>> return 1
>> # _c_serialize_helper_insert_padding()
>>
>> def _c_serialize_helper_switch(context, self, complex_name,
>> code_lines, temp_vars,
>> @@ -1001,21 +1009,23 @@ def _c_serialize_helper_fields(context, self,
>>
>> # fields with variable size
>> else:
>> if field.type.is_pad:
>> # Variable length pad is <pad align= />
>> code_lines.append('%s xcb_align_to = %d;' % (space, field.type.align))
>> count += _c_serialize_helper_insert_padding(context, code_lines, space,
>> - self.c_var_followed_by_fixed_fields)
>> + self.c_var_followed_by_fixed_fields,
>> + is_case_or_bitcase)
>> continue
>> else:
>> # switch/bitcase: always calculate padding before and after variable sized fields
>> if need_padding or is_case_or_bitcase:
>> count += _c_serialize_helper_insert_padding(context, code_lines, space,
>> - self.c_var_followed_by_fixed_fields)
>> + self.c_var_followed_by_fixed_fields,
>> + is_case_or_bitcase)
>>
>> value, length = _c_serialize_helper_fields_variable_size(context, self, field,
>> code_lines, temp_vars,
>> space, prefix)
>> prev_field_was_variable = True
>>
>> # save (un)serialization C code
>> @@ -1096,15 +1106,15 @@ def _c_serialize_helper(context, complex_type,
>> code_lines.append('%s xcb_buffer_len += xcb_block_len;' % space)
>> code_lines.append('%s xcb_block_len = 0;' % space)
>>
>> count += _c_serialize_helper_fields(context, self,
>> code_lines, temp_vars,
>> space, prefix, False)
>> # "final padding"
>> - count += _c_serialize_helper_insert_padding(context, code_lines, space, False)
>> + count += _c_serialize_helper_insert_padding(context, code_lines, space, False, self.is_switch)
>>
>> return count
>> # _c_serialize_helper()
>>
>> def _c_serialize(context, self):
>> """
>> depending on the context variable, generate _serialize(), _unserialize(), _unpack(), or _sizeof()
>> @@ -1166,14 +1176,16 @@ def _c_serialize(context, self):
>> _c(' unsigned int xcb_out_pad = -sizeof(%s) & 3;', self.c_type)
>> _c(' unsigned int xcb_buffer_len = sizeof(%s) + xcb_out_pad;', self.c_type)
>> _c(' unsigned int xcb_align_to = 0;')
>> else:
>> _c(' char *xcb_out = *_buffer;')
>> _c(' unsigned int xcb_buffer_len = 0;')
>> _c(' unsigned int xcb_align_to = 0;')
>> + if self.is_switch:
>> + _c(' unsigned int xcb_padding_offset = ((size_t)xcb_out) & 7;')
>> prefix = [('_aux', '->', self)]
>> aux_ptr = 'xcb_out'
>>
>> elif context in ('unserialize', 'unpack'):
>> _c(' char *xcb_tmp = (char *)_buffer;')
>> if not self.is_switch:
>> if not self.c_var_followed_by_fixed_fields:
>> @@ -1189,14 +1201,16 @@ def _c_serialize(context, self):
>> aux_var = '(*_aux)' # unserialize: double pointer (!)
>> prefix = [(aux_var, '->', self)]
>> aux_ptr = '*_aux'
>> _c(' unsigned int xcb_buffer_len = 0;')
>> _c(' unsigned int xcb_block_len = 0;')
>> _c(' unsigned int xcb_pad = 0;')
>> _c(' unsigned int xcb_align_to = 0;')
>> + if self.is_switch:
>> + _c(' unsigned int xcb_padding_offset = ((size_t)_buffer) & 7;')
>>
>> elif 'sizeof' == context:
>> param_names = [p[2] for p in params]
>> if self.is_switch:
>> # switch: call _unpack()
>> _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))
>> @@ -1206,14 +1220,16 @@ def _c_serialize(context, self):
>> # special case: call _unserialize()
>> _c(' return %s(%s, NULL);', self.c_unserialize_name, reduce(lambda x,y: "%s, %s" % (x, y), param_names))
>> _c('}')
>> return
>> else:
>> _c(' char *xcb_tmp = (char *)_buffer;')
>> prefix = [('_aux', '->', self)]
>> + if self.is_switch:
>> + _c(' unsigned int xcb_padding_offset = 0;')
>>
>> count = _c_serialize_helper(context, self, code_lines, temp_vars, prefix=prefix)
>> # update variable size fields (only important for context=='serialize'
>> variable_size_fields = count
>> if 'serialize' == context:
>> temp_vars.append(' unsigned int xcb_pad = 0;')
>> temp_vars.append(' char xcb_pad0[3] = {0, 0, 0};')
>> --
>> 2.0.1
>>
>> _______________________________________________
>> Xcb mailing list
>> Xcb at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/xcb
>
More information about the Xcb
mailing list