Discussion:
[Xcb] [PATCH libxcb v2 3/4] c_client: Add support for lists of FDs
Daniel Stone
2017-06-08 18:41:10 UTC
Permalink
Matching xcbgen changes, add support having a ListType which contains
file descriptors. Use this to send a variable number of FDs to the
server, including when the list size is not fixed.

Signed-off-by: Daniel Stone <***@collabora.com>
---
src/c_client.py | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)

v2: No changes.

diff --git a/src/c_client.py b/src/c_client.py
index f3fa5b1..2213a31 100644
--- a/src/c_client.py
+++ b/src/c_client.py
@@ -2340,12 +2340,30 @@ def _c_request_helper(self, name, void, regular, aux=False, reply_fds=False):
if aux:
_c(' void *xcb_aux%d = 0;' % (idx))
if list_with_var_size_elems:
- _c(' unsigned int i;')
_c(' unsigned int xcb_tmp_len;')
_c(' char *xcb_tmp;')
- num_fds = len([field for field in param_fields if field.isfd])
- if num_fds > 0:
- _c(' int fds[%d];' % (num_fds))
+
+ num_fds_fixed = 0
+ num_fds_expr = []
+ for field in param_fields:
+ if field.isfd:
+ if not field.type.is_list:
+ num_fds_fixed += 1
+ else:
+ num_fds_expr.append(_c_accessor_get_expr(field.type.expr, None))
+
+ if list_with_var_size_elems or len(num_fds_expr) > 0:
+ _c(' unsigned int i;')
+
+ if num_fds_fixed > 0:
+ num_fds_expr.append('%d' % (num_fds_fixed))
+ if len(num_fds_expr) > 0:
+ num_fds = '+'.join(num_fds_expr)
+ _c(' int fds[%s];' % (num_fds))
+ _c(' int fd_index = 0;')
+ else:
+ num_fds = None
+
_c('')

# fixed size fields
@@ -2451,16 +2469,18 @@ def _c_request_helper(self, name, void, regular, aux=False, reply_fds=False):
# no padding necessary - _serialize() keeps track of padding automatically

_c('')
- fd_index = 0
for field in param_fields:
if field.isfd:
- _c(' fds[%d] = %s;', fd_index, field.c_field_name)
- fd_index = fd_index + 1
+ if not field.type.is_list:
+ _c(' fds[fd_index++] = %s;', field.c_field_name)
+ else:
+ _c(' for (i = 0; i < %s; i++)', _c_accessor_get_expr(field.type.expr, None))
+ _c(' fds[fd_index++] = %s[i];', field.c_field_name)

- if num_fds == 0:
+ if not num_fds:
_c(' xcb_ret.sequence = xcb_send_request(c, %s, xcb_parts + 2, &xcb_req);', func_flags)
else:
- _c(' xcb_ret.sequence = xcb_send_request_with_fds(c, %s, xcb_parts + 2, &xcb_req, %d, fds);', func_flags, num_fds)
+ _c(' xcb_ret.sequence = xcb_send_request_with_fds(c, %s, xcb_parts + 2, &xcb_req, %s, fds);', func_flags, num_fds)

# free dyn. all. data, if any
for f in free_calls:
--
2.13.0
Daniel Stone
2017-06-08 18:41:09 UTC
Permalink
For when we have a variable-sized field followed by a fixed field, make
sure we do not serialise non-wire fields.

Signed-off-by: Daniel Stone <***@collabora.com>
---
src/c_client.py | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

v2: Do the same in more places.

diff --git a/src/c_client.py b/src/c_client.py
index 0cbdf30..f3fa5b1 100644
--- a/src/c_client.py
+++ b/src/c_client.py
@@ -458,7 +458,7 @@ def _c_type_setup(self, name, postfix):
field.c_field_const_type = 'const ' + field.c_field_type
self.c_need_aux = True

- if not field.type.fixed_size() and not field.type.is_case_or_bitcase:
+ if not field.type.fixed_size() and not field.type.is_case_or_bitcase and field.wire:
self.c_need_sizeof = True

field.c_iterator_type = _t(field.field_type + ('iterator',)) # xcb_fieldtype_iterator_t
@@ -497,7 +497,7 @@ def _c_type_setup(self, name, postfix):
_c_type_setup(field.type, field.field_type, ())
if field.type.is_list:
_c_type_setup(field.type.member, field.field_type, ())
- if (field.type.nmemb is None):
+ if (field.type.nmemb is None and field.wire):
self.c_need_sizeof = True

if self.c_need_serialize:
@@ -1170,6 +1170,8 @@ def _c_serialize_helper_fields(context, self,
_c_pre.push_indent(space + ' ')

for field in self.fields:
+ if not field.wire:
+ continue
if not field.visible:
if not ((field.wire and not field.auto) or 'unserialize' == context):
continue
@@ -1194,7 +1196,9 @@ def _c_serialize_helper_fields(context, self,

# fields with variable size
else:
- if field.type.is_pad:
+ if not field.wire:
+ continue
+ elif 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, self, code_lines, space,
@@ -2308,7 +2312,7 @@ def _c_request_helper(self, name, void, regular, aux=False, reply_fds=False):
count = 2
if not self.c_var_followed_by_fixed_fields:
for field in param_fields:
- if not field.type.fixed_size():
+ if not field.type.fixed_size() and field.wire:
count = count + 2
if field.type.c_need_serialize:
# _serialize() keeps track of padding automatically
@@ -2379,7 +2383,7 @@ def _c_request_helper(self, name, void, regular, aux=False, reply_fds=False):
count = 4

for field in param_fields:
- if not field.type.fixed_size():
+ if field.wire and not field.type.fixed_size():
_c(' /* %s %s */', field.type.c_type, field.c_field_name)
# default: simple cast to char *
if not field.type.c_need_serialize and not field.type.c_need_sizeof:
--
2.13.0
Daniel Stone
2017-06-08 18:41:08 UTC
Permalink
Add a special case in ListType to support lists of file descriptors,
which also requires explicit support within clients.

File descriptors are not supported as children of other complex types,
e.g. switch.

Signed-off-by: Daniel Stone <***@collabora.com>
---
xcbgen/xtypes.py | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)

v2: No changes.

diff --git a/xcbgen/xtypes.py b/xcbgen/xtypes.py
index c1f5986..1e270ae 100644
--- a/xcbgen/xtypes.py
+++ b/xcbgen/xtypes.py
@@ -42,6 +42,7 @@ class Type(object):
self.is_case_or_bitcase = False
self.is_bitcase = False
self.is_case = False
+ self.is_fd = False
self.required_start_align = Alignment()

# the biggest align value of an align-pad contained in this type
@@ -68,7 +69,7 @@ class Type(object):
'''
raise Exception('abstract fixed_size method not overridden!')

- def make_member_of(self, module, complex_type, field_type, field_name, visible, wire, auto, enum=None):
+ def make_member_of(self, module, complex_type, field_type, field_name, visible, wire, auto, enum=None, is_fd=False):
'''
Default method for making a data type a member of a structure.
Extend this if the data type needs to add an additional length field or something.
@@ -77,7 +78,7 @@ class Type(object):
complex_type is the structure object.
see Field for the meaning of the other parameters.
'''
- new_field = Field(self, field_type, field_name, visible, wire, auto, enum)
+ new_field = Field(self, field_type, field_name, visible, wire, auto, enum, is_fd)

# We dump the _placeholder_byte if any fields are added.
for (idx, field) in enumerate(complex_type.fields):
@@ -217,6 +218,18 @@ tchar = SimpleType(('char',), 1)
tfloat = SimpleType(('float',), 4)
tdouble = SimpleType(('double',), 8)

+class FileDescriptor(SimpleType):
+ '''
+ Derived class which represents a file descriptor.
+ '''
+ def __init__(self):
+ SimpleType.__init__(self, ('int'), 4)
+ self.is_fd = True
+
+ def fixed_size(self):
+ return True
+
+ out = __main__.output['simple']

class Enum(SimpleType):
'''
@@ -310,7 +323,9 @@ class ListType(Type):
type.make_member_of(module, complex_type, lenfield_type, lenfield_name, True, lenwire, False, enum)

# Add ourself to the structure by calling our original method.
- Type.make_member_of(self, module, complex_type, field_type, field_name, visible, wire, auto, enum)
+ if self.member.is_fd:
+ wire = False
+ Type.make_member_of(self, module, complex_type, field_type, field_name, visible, wire, auto, enum, self.member.is_fd)

def resolve(self, module):
if self.resolved:
@@ -532,7 +547,12 @@ class ComplexType(Type):
elif child.tag == 'list':
field_name = child.get('name')
fkey = child.get('type')
- type = ListType(child, module.get_type(fkey), *self.lenfield_parent)
+ if fkey == 'fd':
+ ftype = FileDescriptor()
+ fkey = 'INT32'
+ else:
+ ftype = module.get_type(fkey)
+ type = ListType(child, ftype, *self.lenfield_parent)
visible = True
elif child.tag == 'switch':
field_name = child.get('name')
--
2.13.0
Daniel Stone
2017-06-08 18:41:11 UTC
Permalink
Bumping to version 1.1, add support for:
- querying formats and modifiers supported by the server
- creating (and receiving) multi-planar buffers
- creating (and receiving) buffers with modifiers
- mapping between dma-fence FDs and Present fences

Signed-off-by: Daniel Stone <***@collabora.com>
---
src/dri3.xml | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/src/dri3.xml b/src/dri3.xml
index 608af31..7b09284 100644
--- a/src/dri3.xml
+++ b/src/dri3.xml
@@ -23,7 +23,7 @@ OF THIS SOFTWARE.
-->

<xcb header="dri3" extension-xname="DRI3" extension-name="DRI3"
- major-version="1" minor-version="0">
+ major-version="1" minor-version="1">
<import>xproto</import>

<!-- Types -->
@@ -94,4 +94,94 @@ OF THIS SOFTWARE.
</reply>
</request>

+ <!-- v1.1 -->
+ <request name="GetSupportedFormats" opcode="6">
+ <field type="CARD32" name="window" />
+ <reply>
+ <pad bytes="1" />
+ <field type="CARD32" name="num_formats" />
+ <pad bytes="20" />
+ <list type="CARD32" name="formats">
+ <fieldref>num_formats</fieldref>
+ </list>
+ </reply>
+ </request>
+
+ <request name="GetSupportedModifiers" opcode="7">
+ <field type="CARD32" name="window" />
+ <field type="CARD32" name="format"/>
+ <reply>
+ <pad bytes="1" />
+ <field type="CARD32" name="format" />
+ <field type="CARD32" name="num_modifiers" />
+ <pad bytes="16" />
+ <list type="CARD32" name="modifiers"> <!-- CARD32 hi followed by lo -->
+ <op op="*">
+ <fieldref>num_modifiers</fieldref>
+ <value>2</value>
+ </op>
+ </list>
+ </reply>
+ </request>
+
+ <request name="PixmapFromBuffers" opcode="8">
+ <field type="PIXMAP" name="pixmap" />
+ <field type="DRAWABLE" name="drawable" />
+ <field type="CARD8" name="num_buffers" />
+ <pad bytes="3" />
+ <field type="CARD16" name="width" />
+ <field type="CARD16" name="height" />
+ <field type="CARD32" name="stride0" />
+ <field type="CARD32" name="offset0" />
+ <field type="CARD32" name="stride1" />
+ <field type="CARD32" name="offset1" />
+ <field type="CARD32" name="stride2" />
+ <field type="CARD32" name="offset2" />
+ <field type="CARD32" name="stride3" />
+ <field type="CARD32" name="offset3" />
+ <field type="CARD32" name="format" />
+ <field type="CARD32" name="modifier_hi" />
+ <field type="CARD32" name="modifier_lo" />
+ <list type="fd" name="buffers">
+ <fieldref>num_buffers</fieldref>
+ </list>
+ </request>
+
+ <request name="BuffersFromPixmap" opcode="9">
+ <field type="PIXMAP" name="pixmap" />
+ <reply>
+ <field type="CARD8" name="nfd"/>
+ <field type="CARD16" name="width" />
+ <field type="CARD16" name="height" />
+ <field type="CARD32" name="format" />
+ <field type="CARD32" name="modifier_hi" />
+ <field type="CARD32" name="modifier_lo" />
+ <pad bytes="8"/>
+ <list type="CARD32" name="strides">
+ <fieldref>nfd</fieldref>
+ </list>
+ <list type="CARD32" name="offsets">
+ <fieldref>nfd</fieldref>
+ </list>
+ <list type="fd" name="buffers">
+ <fieldref>nfd</fieldref>
+ </list>
+ </reply>
+ </request>
+
+ <request name="FenceFromDMAFenceFD" opcode="10">
+ <field type="DRAWABLE" name="drawable" />
+ <field type="CARD32" name="fence"/>
+ <fd name="fence_fd"/>
+ </request>
+
+ <request name="DMAFenceFDFromFence" opcode="11">
+ <field type="DRAWABLE" name="drawable" />
+ <field type="CARD32" name="fence"/>
+ <reply>
+ <field type="CARD8" name="nfd"/>
+ <fd name="fence_fd" />
+ <pad bytes="24"/>
+ </reply>
+ </request>
</xcb>
--
2.13.0
Eric Anholt
2017-06-17 01:11:50 UTC
Permalink
Post by Daniel Stone
Matching xcbgen changes, add support having a ListType which contains
file descriptors. Use this to send a variable number of FDs to the
server, including when the list size is not fixed.
I've verified that patch 1 and 2 don't change the generated code, that
this patch only cosmetically changes the code, and that the generated
code for patch 4 looks reasonable.

I find the python to be pretty hairy, but it seems fine, and it's really
the generated code that matters, so patch 1-3 are:

Reviewed-by: Eric Anholt <***@anholt.net>
Daniel Stone
2017-07-20 17:15:23 UTC
Permalink
Hi Eric,
Post by Eric Anholt
Post by Daniel Stone
Matching xcbgen changes, add support having a ListType which contains
file descriptors. Use this to send a variable number of FDs to the
server, including when the list size is not fixed.
I've verified that patch 1 and 2 don't change the generated code, that
this patch only cosmetically changes the code, and that the generated
code for patch 4 looks reasonable.
I find the python to be pretty hairy, but it seems fine, and it's really
Thanks for the review! FWIW, I find the Python quite hairy as well,
but after a couple of attempted reworks, couldn't find anything which
worked better.

Uli, would you be OK pushing these first 3 support patches, or would
you prefer I did, or ... ?

Cheers,
Daniel
Uli Schlachter
2017-07-22 11:12:45 UTC
Permalink
Post by Daniel Stone
Hi Eric,
Post by Eric Anholt
Post by Daniel Stone
Matching xcbgen changes, add support having a ListType which contains
file descriptors. Use this to send a variable number of FDs to the
server, including when the list size is not fixed.
I've verified that patch 1 and 2 don't change the generated code, that
this patch only cosmetically changes the code, and that the generated
code for patch 4 looks reasonable.
I find the python to be pretty hairy, but it seems fine, and it's really
Thanks for the review! FWIW, I find the Python quite hairy as well,
but after a couple of attempted reworks, couldn't find anything which
worked better.
Uli, would you be OK pushing these first 3 support patches, or would
you prefer I did, or ... ?
I'd think that it is less work overall if you push them since you
already have them in Git. Also, I usually try to stay away from the
Python code around XCB. :-)

So, feel free to push them.

Uli
--
If you have to type the letters "A-E-S" into your source code, you're
doing it wrong.
Daniel Stone
2017-07-22 12:20:38 UTC
Permalink
Hi Uli,
Post by Uli Schlachter
Post by Daniel Stone
Post by Eric Anholt
I've verified that patch 1 and 2 don't change the generated code, that
this patch only cosmetically changes the code, and that the generated
code for patch 4 looks reasonable.
I find the python to be pretty hairy, but it seems fine, and it's really
Thanks for the review! FWIW, I find the Python quite hairy as well,
but after a couple of attempted reworks, couldn't find anything which
worked better.
Uli, would you be OK pushing these first 3 support patches, or would
you prefer I did, or ... ?
I'd think that it is less work overall if you push them since you
already have them in Git. Also, I usually try to stay away from the
Python code around XCB. :-)
So, feel free to push them.
Thanks a lot! I've pushed both sets of preparatory patches now, so
there's just the actual DRI3 protocol pending.

To ssh://git.freedesktop.org/git/xcb/proto
1c05de5..9df4ead 9df4eadb482be083c66c856e9c7f38331d50a2b4 -> master

To ssh://git.freedesktop.org/git/xcb/libxcb
d10194a..a3e9821 master -> master

Cheers,
Daniel

Loading...