[Xcb] [PATCH libxcb 10/7] c_client.py: don't generate _aux() request functions

Ran Benita ran234 at gmail.com
Tue Feb 25 04:11:37 PST 2014


Currently, for requests which take a parameter which needs to be
serialized (like some complex struct), there are two types of functions
generated:
    xbc_<request_name>{,checked,unchecked}
    xbc_<request_name>_aux{,checked,unchecked}
The difference between the aux and non-aux versions is how they handle
the complex argument(s):

- The aux version takes a type-safe instance of the struct, and
  internally calls the _serialize() function for this struct, which
  translates it to its wire representation.

- The non-aux version instead takes a void*, unpacks it to the struct in
  order to calculate the sizeof, and then sends it directly.

The non-aux version is not type-safe and expects the user to lay out the
data manually. People also don't know the aux version even exists, and
thus use the non-aux version incorrectly.

Therefore, remove the non-aux versions and drop the _aux prefix
entirely, and make the code-generator worry about one less obscurity on
the way.

The affected functions are:

xcb_sync_create_alarm
xcb_sync_change_alarm
    It was recently fixed in proto to use a switch, which caused the
    _aux function to be generated:
    http://cgit.freedesktop.org/xcb/proto/commit/?id=e6a246e50e62cbcba33d0e1d2371e69e6e089383

    It is used (incorrectly) by KDE kwin. They have added a workaround
    because it is evidently not working (should have used the aux
    version):
    https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/008ac5efabfb99f04813e5dad29c2d0a92d13fc5
    In my tests it continues to not-work as before, but with this change
    they will get a type error, which would hopefully prompt them to fix
    the call.

    Finally, I wasn't able to find anyone using the _aux version, except
    this Weston patch which wasn't applied:
    http://lists.freedesktop.org/archives/wayland-devel/2013-July/010416.html

xcb_input_change_device_property
xcb_input_xi_change_property
    xinput is still disabled by default, and I'm quite sure these
    functions aren't used in any case.

xcb_xkb_select_events
    This function is used by Qt5, xkbcommon-x11 and similar others. All
    of them pass NULL to the last (aux) argument, and they continue to
    work correctly with this change.

xcb_xkb_set_map
xcb_xkb_set_names
    These functions are not used anywhere.

Cc: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
Cc: Martin Gräßlin <mgraesslin at kde.org>
Signed-off-by: Ran Benita <ran234 at gmail.com>
---
 src/c_client.py | 75 ++++++++++++++++++++-------------------------------------
 1 file changed, 26 insertions(+), 49 deletions(-)

diff --git a/src/c_client.py b/src/c_client.py
index f2bef2f..8690a59 100644
--- a/src/c_client.py
+++ b/src/c_client.py
@@ -305,13 +305,9 @@ def _c_type_setup(self, name, postfix):
     self.c_cookie_type = _t(name + ('cookie',))
     self.c_reply_fds_name = _n(name + ('reply_fds',))
 
-    self.c_need_aux = False
     self.c_need_serialize = False
     self.c_need_sizeof = False
 
-    self.c_aux_name = _n(name + ('aux',))
-    self.c_aux_checked_name = _n(name + ('aux', 'checked'))
-    self.c_aux_unchecked_name = _n(name + ('aux', 'unchecked'))
     self.c_serialize_name = _n(name + ('serialize',))
     self.c_unserialize_name = _n(name + ('unserialize',))
     self.c_unpack_name = _n(name + ('unpack',))
@@ -357,7 +353,6 @@ def _c_type_setup(self, name, postfix):
             if field.type.is_switch:
                 field.c_pointer = '*'
                 field.c_field_const_type = 'const ' + field.c_field_type
-                self.c_need_aux = True
             elif not field.type.fixed_size() and not field.type.is_bitcase:
                 self.c_need_sizeof = True
 
@@ -1763,7 +1758,7 @@ def c_union(self, name):
     _c_complex(self)
     _c_iterator(self, name)
 
-def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_fds=False):
+def _c_request_helper(self, name, cookie_type, void, regular, reply_fds=False):
     '''
     Declares a request function.
     '''
@@ -1802,11 +1797,12 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
     func_ext_global = '&' + _ns.c_ext_global_name if _ns.is_ext else '0'
 
     # What our function name is
-    func_name = self.c_request_name if not aux else self.c_aux_name
     if checked:
-        func_name = self.c_checked_name if not aux else self.c_aux_checked_name
-    if unchecked:
-        func_name = self.c_unchecked_name if not aux else self.c_aux_unchecked_name
+        func_name = self.c_checked_name
+    elif unchecked:
+        func_name = self.c_unchecked_name
+    else:
+        func_name = self.c_request_name
 
     param_fields = []
     wire_fields = []
@@ -1827,8 +1823,6 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
 
     for field in param_fields:
         c_field_const_type = field.c_field_const_type
-        if field.type.c_need_serialize and not aux:
-            c_field_const_type = "const void"
         if len(c_field_const_type) > maxtypelen:
             maxtypelen = len(c_field_const_type)
         if field.type.is_list and not field.type.member.fixed_size():
@@ -1850,7 +1844,7 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
     if hasattr(self, "doc") and self.doc:
         for field in param_fields:
             # XXX: hard-coded until we fix xproto.xml
-            base_func_name = self.c_request_name if not aux else self.c_aux_name
+            base_func_name = self.c_request_name
             if base_func_name == 'xcb_change_gc' and field.c_field_name == 'value_mask':
                 field.enum = 'GC'
             elif base_func_name == 'xcb_change_window_attributes' and field.c_field_name == 'value_mask':
@@ -1915,9 +1909,6 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
         count = count - 1
         c_field_const_type = field.c_field_const_type
         c_pointer = field.c_pointer
-        if field.type.c_need_serialize and not aux:
-            c_field_const_type = "const void"
-            c_pointer = '*'
         spacing = ' ' * (maxtypelen - len(c_field_const_type))
         comma = ',' if count else ');'
         _h('%s%s%s %s%s  /**< */%s', func_spacing, c_field_const_type,
@@ -1951,9 +1942,8 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
         _c('    /* in the protocol description, variable size fields are followed by fixed size fields */')
         _c('    void *xcb_aux = 0;')
 
-
-    for idx, f in enumerate(serial_fields):
-        if aux:
+    for idx, field in enumerate(serial_fields):
+        if field.type.c_need_serialize:
             _c('    void *xcb_aux%d = 0;' % (idx))
     if list_with_var_size_elems:
         _c('    unsigned int i;')
@@ -1979,7 +1969,7 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
                 else:
                     _c('    memcpy(xcb_out.%s, %s, %d);', field.c_field_name, field.c_field_name, field.type.nmemb)
 
-    def get_serialize_args(type_obj, c_field_name, aux_var, context='serialize'):
+    def get_serialize_args(type_obj, c_field_name, aux_var, context):
         serialize_args = get_serialize_params(context, type_obj,
                                               c_field_name,
                                               aux_var)[2]
@@ -2023,22 +2013,19 @@ def _c_request_helper(self, name, cookie_type, void, regular, aux=False, reply_f
                     else:
                         # not supposed to happen
                         raise Exception("unhandled variable size field %s" % field.c_field_name)
-                else:
-                    if not aux:
-                        _c('    xcb_parts[%d].iov_base = (char *) %s;', count, field.c_field_name)
+                elif field.type.c_need_serialize:
                     idx = serial_fields.index(field)
                     aux_var = '&xcb_aux%d' % idx
-                    context = 'serialize' if aux else 'sizeof'
+                    serialize_args = get_serialize_args(field.type, aux_var, field.c_field_name, 'serialize')
                     _c('    xcb_parts[%d].iov_len =', count)
-                    if aux:
-                        serialize_args = get_serialize_args(field.type, aux_var, field.c_field_name, context)
-                        _c('      %s (%s);', field.type.c_serialize_name, serialize_args)
-                        _c('    xcb_parts[%d].iov_base = xcb_aux%d;' % (count, idx))
-                        free_calls.append('    free(xcb_aux%d);' % idx)
-                    else:
-                        serialize_args = get_serialize_args(field.type, field.c_field_name, aux_var, context)
-                        func_name = field.type.c_sizeof_name
-                        _c('      %s (%s);', func_name, serialize_args)
+                    _c('      %s (%s);', field.type.c_serialize_name, serialize_args)
+                    _c('    xcb_parts[%d].iov_base = xcb_aux%d;' % (count, idx))
+                    free_calls.append('    free(xcb_aux%d);' % idx)
+                elif field.type.c_need_sizeof:
+                    _c('    xcb_parts[%d].iov_base = (char *) %s;', count, field.c_field_name)
+                    serialize_args = get_serialize_args(field.type, field.c_field_name, '_aux', 'sizeof')
+                    _c('    xcb_parts[%d].iov_len =', count)
+                    _c('      %s (%s);', field.type.c_sizeof_name, serialize_args)
 
                 count += 1
                 if not (field.type.c_need_serialize or field.type.c_need_sizeof):
@@ -2212,10 +2199,10 @@ def _c_cookie(self, name):
     _h('    unsigned int sequence; /**<  */')
     _h('} %s;', self.c_cookie_type)
 
-def _man_request(self, name, cookie_type, void, aux):
+def _man_request(self, name, cookie_type, void):
     param_fields = [f for f in self.fields if f.visible]
 
-    func_name = self.c_request_name if not aux else self.c_aux_name
+    func_name = self.c_request_name
 
     def create_link(linkname):
         name = 'man/%s.%s' % (linkname, section)
@@ -2249,15 +2236,12 @@ def _man_request(self, name, cookie_type, void, aux):
         c_pointer = field.c_pointer
         if c_pointer == ' ':
             c_pointer = ''
-        if field.type.c_need_serialize and not aux:
-            c_field_const_type = "const void"
-            c_pointer = '*'
         comma = ', ' if count else ');'
         prototype += '%s\\ %s\\fI%s\\fP%s' % (c_field_const_type, c_pointer, field.c_field_name, comma)
 
     f.write('.SS Request function\n')
     f.write('.HP\n')
-    base_func_name = self.c_request_name if not aux else self.c_aux_name
+    base_func_name = self.c_request_name
     f.write('%s \\fB%s\\fP(xcb_connection_t\\ *\\fIconn\\fP, %s\n' % (cookie_type, base_func_name, prototype))
     create_link('%s_%s' % (base_func_name, ('checked' if void else 'unchecked')))
     if not void:
@@ -2751,11 +2735,8 @@ def c_request(self, name):
         _c_complex(self.reply)
         # Request prototypes
         has_fds = _c_reply_has_fds(self.reply)
-        _c_request_helper(self, name, self.c_cookie_type, False, True, False, has_fds)
-        _c_request_helper(self, name, self.c_cookie_type, False, False, False, has_fds)
-        if self.c_need_aux:
-            _c_request_helper(self, name, self.c_cookie_type, False, True, True, has_fds)
-            _c_request_helper(self, name, self.c_cookie_type, False, False, True, has_fds)
+        _c_request_helper(self, name, self.c_cookie_type, False, True, has_fds)
+        _c_request_helper(self, name, self.c_cookie_type, False, False, has_fds)
         # Reply accessors
         _c_accessors(self.reply, name + ('reply',), name)
         _c_reply(self, name)
@@ -2765,14 +2746,10 @@ def c_request(self, name):
         # Request prototypes
         _c_request_helper(self, name, 'xcb_void_cookie_t', True, False)
         _c_request_helper(self, name, 'xcb_void_cookie_t', True, True)
-        if self.c_need_aux:
-            _c_request_helper(self, name, 'xcb_void_cookie_t', True, False, True)
-            _c_request_helper(self, name, 'xcb_void_cookie_t', True, True, True)
 
     # We generate the manpage afterwards because _c_type_setup has been called.
-    # TODO: what about aux helpers?
     cookie_type = self.c_cookie_type if self.reply else 'xcb_void_cookie_t'
-    _man_request(self, name, cookie_type, not self.reply, False)
+    _man_request(self, name, cookie_type, not self.reply)
 
 def c_event(self, name):
     '''
-- 
1.9.0



More information about the Xcb mailing list