[Xcb] xinput: make ListInputDevices work, sumof with nested expr, ...

[Ran Benita]

On Mon, Oct 13, 2014 at 11:59:20AM +0200, Christian Linhart wrote:
> On 10/12/14 20:39, Ran Benita wrote:
> > On Wed, Sep 03, 2014 at 01:14:23PM +0200, Christian Linhart wrote:
> [...]
> >> * make sumof more flexible. Especially support a nested expression which is evaluated
> >>   in the scope of each list-element which is iterated to compute sumof.
> >>   Example:
> >>         <sumof ref="devices">
> >>             <fieldref>num_class_info</fieldref>
> >>         </sumof>
> > 
> > xcb_sumof() is this:
> > 
> >   int xcb_sumof(uint8_t *list, int len)
> >   {
> >     int i, s = 0;
> >     for(i=0; i<len; i++) {
> >       s += *list;
> >       list++;
> >     }
> >     return s;
> >   }
> > 
> > So it assumes the list is of CARD8's - it is not polymorphic in any
> > way. It looks OK for the added sumof in this series, but I'm not sure it
> > matches the changes in the generator?
> 
> The function xcb_sumof is not used anymore with my changes in the generator.
> Rather, an explicit for-loop is added before the expression that needs the sum.
> (with the help of several temporary variables).
> 
> Generating that explicit code is easier than to make C do the needed polymorphic stuff.

Huh, shouldn't comment on a series before reading all of it!

> Here's an example:
> The sumof-code is between the comments /* sumof start */ and /* sumof end. Result is in xcb_pre_tmp_7 */.
> 
> ====================
> xcb_input_list_input_devices_infos_length (const xcb_input_list_input_devices_reply_t *R  /**< */)
> {
>     int xcb_pre_tmp_5; /* sumof length */
>     int xcb_pre_tmp_6; /* sumof loop counter */
>     int64_t xcb_pre_tmp_7; /* sumof sum */
>     const xcb_input_device_info_t* xcb_pre_tmp_8; /* sumof list ptr */
>     /* sumof start */
>     xcb_pre_tmp_5 = R->devices_len;
>     xcb_pre_tmp_7 = 0;
>     xcb_pre_tmp_8 = xcb_input_list_input_devices_devices(R);
>     for (xcb_pre_tmp_6 = 0; xcb_pre_tmp_6 < xcb_pre_tmp_5; xcb_pre_tmp_6++) {
>         xcb_pre_tmp_7 += xcb_pre_tmp_8->num_class_info;
>         xcb_pre_tmp_8++;
>     }
>     /* sumof end. Result is in xcb_pre_tmp_7 */
>     return xcb_pre_tmp_7;
> }
> ====================
> 
> So, the non-polymorphicness of function xcb_sumof is not an issue because this function is not used anymore.
> (but kept in there for compatibility reasons, for the case that some user-code uses it directly somewhere...)

Yes, this looks like the sanest (only?) way to handle this in C.

Re. compatability, not sure if xcbext.h is considered API, but probably
not a bad idea to keep it as it's very small.

> >> Due to using switch-case, some structs have became obsolete.
> >> (and due to using struct STR from xproto instead of DeviceName because these
> >> are the same and the spec uses STR)
> >>
> >> This patchset contains two patches to remove these obsolete structs.
> >> You may reject these patches due to API-compatibility concerns.
> >> On the other hand, probably nobody has used the structs for request ListInputDevice
> >> anyways before because this request/reply didn't work anyways in the previous form.
> > 
> > I think we can break xinput, it is not declared stable yet and is
> > disabled by default. However, it appears like some projects already use
> > it:
> > http://git.enlightenment.org/legacy/ecore.git/tree/src/lib/ecore_x/xcb/ecore_xcb_input.c
> > So I'm not sure...
> 
> Hmmm. If we are not sure we shouldn't remove those structs. 
> (better be safe than sorry where the safe alternative is OK, too)
> But, probably these structs should be marked as deprecated somehow.
> 
> BTW, There are many more obsolete structs. ( primarily from my other changes where I use switch-case. )
> I didn't generate patches for removing those because I wanted to wait for the discussion for this patchset.
> 
> So I suggest the following procedure:
> * drop patches "proto 6/7" and "proto 7/7" now
> 	(I'll update my patch-repo accordingly. I'll send another message when that repo is updated. )

Yes, that will be the safe thing to do. Probably nobody's using them,
especially if the request is broken. But we had some issues with xkb in
the past..

> * after all my of patches are merged:
> 	mark all obsolete structs as deprecated:
> 	- with a comment?
> 	- or with a xml-attribute that is translated to some comment in the C-code,
> 	  and possibly with some macro that expands to "__declspec( deprecated )" or "__attribute__(deprecated)" 
> 	  depending on the compiler used.
> 
> What do you think?

Both sound OK to me. They can also be changed to typedefs maybe.

Ran

> Chris
> 
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb