[Xcb] [PATCH libxcb 2/6] generator: fix absname for fields with only iterator function

[Ran Benita]

On Mon, Oct 13, 2014 at 03:51:50PM +0200, Christian Linhart wrote:
> On 10/12/14 20:24, Ran Benita wrote:
> > On Wed, Sep 03, 2014 at 01:22:37PM +0200, Christian Linhart wrote:
> >> Fix _c_helper_absolute_name for fields which can only be
> >> accessed with an iterator function.
> >> These are lists with var-sized structs as members.
> > 
> > This doesn't sound right. Does the caller know how to handle each
> > iterator? 
> Yes, but the caller has to find that out by itself, 
> which is not ideal because it could lead to inconsistencies.
> Example is in patch "libxcb 6/6" of this patchset.
> 
> I have thought about that when implementing it.
> An alternative would be to return meta-info in addition to the C-expression-string.
> That meta-info can tell the caller that this is an iterator.
> 
> But that would change the caller interface of that function
> and required a lot of changes.
> 
> Now I have a new idea which is cleaner:
> We could use two functions.
> 1. One function which returns meta-info, and that function will be used by the sumof code.
> 
> 2. A compatibility-wrapper function which calls the first function 
>    but only returns the expression-string
>    and throws an exception when this is an iterator, so it prevents
>    that an iterator-access is inserted somewhere where it shouldn't.
> 
> > Where it is used? I see that the code is reached, but this
> > it doesn't seem to reach the generated output anywhere (with this
> > libxcb+proto patchset), as opposed to the accessors in the `else` branch.
> It is used in sumof over lists with varsized members.
> 
> As far as I can remember we do not have such lists in the xml with all of my changes.
> Therefore, it does not reach the generated output as you have observed.
> 
> But I accidentally assumed for some time that we have such lists, so I implemented support for it, and also tested it.
> I thought that this feature might be useful some time in the future, so I have included it in that patchset.
> 
> But I am OK with skipping it.
> 
> If we skip this patch, we also have to skip patch "libxcb 6/6" of this patchset.
> 
> What do you think:
> Skip it or improve it as outlined above?

That's easy - skip it :) If we find out eventually that it *is* needed
by xinput or xkb, we can return to it.

Ran

> Chris
> 
> 
> 
> > 
> > So can we skip this patch? If not, why is it needed?
> > 
> > Ran
> > 
> >> ---
> >>  src/c_client.py | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/c_client.py b/src/c_client.py
> >> index 488f0e6..35c975f 100644
> >> --- a/src/c_client.py
> >> +++ b/src/c_client.py
> >> @@ -464,15 +464,20 @@ def _c_helper_absolute_name(prefix, field=None):
> >>                  ( field.type.is_list and not field.type.fixed_size() )
> >>                  or
> >>                  ( field.prev_varsized_field is not None
> >>                    or not field.type.fixed_size()
> >>                  )
> >>          )
> >>      ):
> >> -        prefix_str = field.c_accessor_name + "(" + prefix_str_without_lastsep + ")";
> >> +        if field.type.is_list and not field.type.member.fixed_size():
> >> +            #only accessible by iterator
> >> +            prefix_str = field.c_iterator_name + "(" + prefix_str_without_lastsep + ")";
> >> +        else:
> >> +            #only accessible by access function
> >> +            prefix_str = field.c_accessor_name + "(" + prefix_str_without_lastsep + ")";
> >>  
> >>      return prefix_str
> >>  # _c_absolute_name
> >>  
> >>  def _c_helper_field_mapping(complex_type, prefix, flat=False):
> >>      """
> >>      generate absolute names, based on prefix, for all fields starting from complex_type
> >> -- 
> >> 2.0.1
> >>
> >> _______________________________________________
> >> Xcb mailing list
> >> Xcb at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/xcb
> > 
>