| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
See the description in the randrproto repository.
Add lease notify event
Signed-off-by: Keith Packard <keithp@keithp.com>
|
|
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 <daniels@collabora.com>
|
|
The last item from this TODO list has been implemented
by the commit "SendExtensionEvent uses eventstruct":
https://cgit.freedesktop.org/xcb/proto/commit/?id=97fbba25baf241cbcda882739db423b1d36e440a
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Use the eventstruct element to define the SendExtensionEvent request
instead of using a byte-array.
This has the advantage that the structure of the data to be sent
is now defined in the xml definition.
This can be used by generators to make sending events safer.
Such as avoiding type-casts in C.
It can also be useful for protocol analyzers and tools like that
because the structure of the sent events is now defined in the xml-definition.
Tested-by: Christian Linhart <chris@demorecorder.com>
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
eventstruct is a new xml element that allows to use events
as part of requests.
This is, e.g., needed by the SendExtensionEvent request
of the XINPUT extension.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
member event_type_base in struct InputClassInfo is a CARD8.
This patch replaces the type with the new typedef EventTypeBase
which is defined as a CARD8.
This does not alter the ABI or API for that struct because
typedefs do not appear in the ABI.
And C handles typedefs as aliases.
So there'll be no API issues as well.
The libxcb generator adds additional functions that implement
an iterator over this type.
This is harmless because adding code is ABI/API compatible.
The purpose of this change is to add more semantic info to this
member variable of this struct.
This helps with xcb-based generators or automatic code-checkers
and things like that.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Set the lenfield of a sumof expression object, so that
libxcb or other generators won't need to look it up.
This object is trivially available in the parser but
needs a complex lookup in generators.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Fields AuthorizationProtocolName and AuthorizationProtocolData of
SetupRequest should be padded:
http://www.x.org/releases/current/doc/xproto/x11protocol.html#Encoding::Connection_Setup
The problem was discovered by github user pphaneuf while trying to use xgb
to write his own implementation of the connection handshake. Neither xgb
nor xcb actually use code generated for SetupRequest for the handshake,
which is why this bug went unnoticed.
https://github.com/BurntSushi/xgb/issues/24
Alessandro Arzilli.
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
Here is an attempt at updating the schema to add serialize attribute to pad
element and required_start_align element.
Not sure if I've added required_start_align element in the right place. The
default case in the switch element is removed as it doesn't seem to be used, and
otherwise makes the schema ambiguous.
$ make check
Making check in src
make[1]: Entering directory '/jhbuild/x86_64-pc-cygwin/build/xcb/proto/src'
make check-local
make[2]: Entering directory '/jhbuild/x86_64-pc-cygwin/build/xcb/proto/src'
/usr/bin/xmllint --noout --schema /jhbuild/checkout/xcb/proto/src/xcb.xsd
/jhbuild/checkout/xcb/proto/src/*.xml
/jhbuild/checkout/xcb/proto/src/bigreq.xml validates
/jhbuild/checkout/xcb/proto/src/composite.xml validates
/jhbuild/checkout/xcb/proto/src/damage.xml validates
/jhbuild/checkout/xcb/proto/src/dpms.xml validates
/jhbuild/checkout/xcb/proto/src/dri2.xml validates
/jhbuild/checkout/xcb/proto/src/dri3.xml validates
/jhbuild/checkout/xcb/proto/src/ge.xml validates
Element 'required_start_align': This element is not expected. Expected is one of ( pad, field, list, fd ).
Element 'required_start_align': This element is not expected. Expected is one of ( pad, field, list, fd ).
Element 'required_start_align': This element is not expected. Expected is one of ( pad, field, list, fd ).
Element 'required_start_align': This element is not expected. Expected is one of ( pad, field, list, fd ).
/jhbuild/checkout/xcb/proto/src/glx.xml fails to validate
Element 'required_start_align': This element is not expected. Expected is one of ( pad, field, list, fd, exprfield, switch, reply, doc ).
Element 'required_start_align': This element is not expected. Expected is one of ( pad, field, list, fd, exprfield, switch, reply, doc ).
Element 'required_start_align': This element is not expected. Expected is one of ( pad, field, list, fd, doc ).
Element 'required_start_align': This element is not expected. Expected is one of ( pad, field, list, fd, doc ).
/jhbuild/checkout/xcb/proto/src/present.xml fails to validate
/jhbuild/checkout/xcb/proto/src/randr.xml validates
/jhbuild/checkout/xcb/proto/src/record.xml validates
/jhbuild/checkout/xcb/proto/src/render.xml validates
/jhbuild/checkout/xcb/proto/src/res.xml validates
/jhbuild/checkout/xcb/proto/src/screensaver.xml validates
/jhbuild/checkout/xcb/proto/src/shape.xml validates
/jhbuild/checkout/xcb/proto/src/shm.xml validates
/jhbuild/checkout/xcb/proto/src/sync.xml validates
/jhbuild/checkout/xcb/proto/src/xc_misc.xml validates
/jhbuild/checkout/xcb/proto/src/xevie.xml validates
/jhbuild/checkout/xcb/proto/src/xf86dri.xml validates
/jhbuild/checkout/xcb/proto/src/xf86vidmode.xml validates
/jhbuild/checkout/xcb/proto/src/xfixes.xml validates
/jhbuild/checkout/xcb/proto/src/xinerama.xml validates
Element 'required_start_align': This element is not expected. Expected is one of ( bitcase, case, pad, field, list, fd ).
Element 'required_start_align': This element is not expected. Expected is one of ( bitcase, case, pad, field, list, fd ).
Element 'required_start_align': This element is not expected. Expected is one of ( bitcase, case, pad, field, list, fd ).
/jhbuild/checkout/xcb/proto/src/xinput.xml fails to validate
Element 'pad', attribute 'serialize': The attribute 'serialize' is not allowed.
Element 'pad', attribute 'serialize': The attribute 'serialize' is not allowed.
Element 'pad', attribute 'serialize': The attribute 'serialize' is not allowed.
Element 'pad', attribute 'serialize': The attribute 'serialize' is not allowed.
Element 'pad', attribute 'serialize': The attribute 'serialize' is not allowed.
/jhbuild/checkout/xcb/proto/src/xkb.xml fails to validate
/jhbuild/checkout/xcb/proto/src/xprint.xml validates
/jhbuild/checkout/xcb/proto/src/xproto.xml validates
/jhbuild/checkout/xcb/proto/src/xselinux.xml validates
/jhbuild/checkout/xcb/proto/src/xtest.xml validates
/jhbuild/checkout/xcb/proto/src/xv.xml validates
/jhbuild/checkout/xcb/proto/src/xvmc.xml validates
Makefile:534: recipe for target 'check-local' failed
Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95494
Reviewd-by: Ran Benita <ran234@gmail.com>
Signed-off-by: Uli Schlachter <psychon@znc.in>
|
|
The specification disagrees with itself, so use the part of the
specification that matches the other implementations.
Reviewed-by: Ran Benita <ran234@gmail.com>
Signed-off-by: Peter Harris <pharris@opentext.com>
|
|
Fixes build with python-3.x.
Signed-off-by: Thomas Klausner <wiz@NetBSD.org>
Signed-off-by: Uli Schlachter <psychon@znc.in>
|
|
At least python-3.5.x complains about this forcefully.
Signed-off-by: Thomas Klausner <wiz@NetBSD.org>
Signed-off-by: Uli Schlachter <psychon@znc.in>
|
|
There were more than 100 commits since the last release. The NEWS file only
provides an overview of these changes.
Signed-off-by: Uli Schlachter <psychon@znc.in>
|
|
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
From now on, due to a patch in libxcb, pads will not be serialized
anymore. This is to maintain ABI-compatibility when adding
explicit align pads.
However, some align pads were already be serialized in prior
official versions of libxcb. Therefore we need a method to
enforce serialization, so we can maintainn ABI compatibility
with that legacy.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Some rework done by Christian Linhart
Signed-off-by: Jaya Tiwari <tiwari.jaya18@gmail.com>
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
These are just minimal adjustments to get xkb through
the checks of the alignment checker.
It is not the big fixup which I have already posted an RFC patch
a while ago.
V2 of this patch:
make indentation consistent with the file
(tabs vs spaces)
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
The verification algorithm basically traverses the protocol-description
of a protocol entity by recursively processing all fields and their
types.
A start-align consists of two numbers:
* the alignment: This is a power of 2, and guarantees that the
address (or protocol position) minus the offset can be divided
by this number.
* the offset: how many bytes the current position is after a
position that can be divided by the alignment.
The algorithm starts with the start-alignment and computes the alignment
of each field from the start-align of the previous field as follows:
* if the previous field is a primitive type then the offset is increased
by the size of the primitive type module the alignment.
If the alignment or offset are incompatible with the primitive type,
then an error is reported.
* if the previous field is a complex type, then it is processed recursively.
* if the previous field is an alignment-pad, then the alignment is
adjusted accordingly, as to be expected by the alignment-pad.
Start-aligns can also be set manually with the xml-element
"required_start_align" which has two attributes: "align" and "offset"
If "offset" is omitted, it is assumed to 0.
All toplevel protocol entities default to 4-byte start-alignment with offset 0.
All non-toplevel complex entities, such as structs, switch, case, ...
do not have a default alignment.
If no alignment is explicitly specified for them, their alignment
is computed by their usage in other entities.
In that case, if they are used with aligments that violate the
alignment requirements of some of their fields, an error is issued.
If they are used with an alignment with non-zero offset,
a warning is issued, which recommends to specify the required_start_align
explicitly. (Reason: I don't want non-zero offsets to be silently
computed automatically. These non-zero offsets have to be reviewed
by a human and specified explicitely to record that this was reviewed
by a human)
If the required_start_align is explicitly specified for an entity
then an error will be issued if it is used with an aligment that's
incompatible with the explicitly specified alignment.
If an entity is used in different contexts with different start-aligns
then those start-aligns are combined to an align which is compatible
with all aligns.
E.g. (align 4, offset 0) and (align 4, offset 2) are combined
to (align 2, offset 0).
Error reports include the relevant context for a misalignment.
For non-toplevel entities this includes the entity-usage stack.
There is some complexity in the algorithm for reducing the size
of the error-reports to include only the relevant info.
This alignment verifier is also a prerequisite for getting
rid of implicit alignment altogether.
(This will then simplify the generated code and make it faster.)
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Especially, these start-aligns have non-zero offsets.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Reviewed-By: Ran Benita <ran234@gmail.com>
|
|
The align pad is needed so that a in list of struct "AttributeInfo",
the 32-bit fields in subsequent "AttributeInfo" are aligned.
(all fields except "name" are 32-bit values)
The xv-protocol-spec does not contain the protocol encoding.
(it specifies the protocol on a higher level)
The X-Server uses function "WriteToClient" to write the list "name"
to the connection:
http://cgit.freedesktop.org/xorg/xserver/tree/Xext/xvdisp.c?id=xorg-server-1.17.99.901#n809
Function "WriteToClient" does 4-byte alignment padding:
http://cgit.freedesktop.org/xorg/xserver/tree/os/io.c?id=xorg-server-1.17.99.901#n804
BTW, With the current server impl, the alignpad is not strictly needed
because the field "size", which describes the length of list "name"
is set to a multiple of 4:
http://cgit.freedesktop.org/xorg/xserver/tree/Xext/xvdisp.c?id=xorg-server-1.17.99.901#n805
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
The align pad is needed so that a in list of struct "EncodingInfo",
the 32-bit fiels in subsequent "EncodingInfo" are aligned.
(the field "encoding" is a 32-bit value, and the field "rate"
contains two 32-bit values)
The xv-protocol-spec does not contain the protocol encoding.
(it specifies the protocol on a higher level)
The X-Server uses function "WriteToClient" to write the list "name"
to the connection:
http://cgit.freedesktop.org/xorg/xserver/tree/Xext/xvdisp.c?id=xorg-server-1.17.99.901#n456
Function "WriteToClient" does 4-byte alignment padding:
http://cgit.freedesktop.org/xorg/xserver/tree/os/io.c?id=xorg-server-1.17.99.901#n804
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
This is needed so that subsequent list-items in a list are aligned
to 4 bytes which is necessary because ListItem contains 32-bit values
of types ATOM and CARD32. (Actually, only the final align-pad
is needed for that.)
The xserver makes sure that the length of both char-lists is a multiple of 4:
http://cgit.freedesktop.org/xorg/xserver/tree/Xext/xselinux_ext.c?id=xorg-server-1.17.99.901#n360
So, instead of a explicit padding we could add something like an assertion
or precondition that the fields "object_context_len" and "data_context_len"
have to be a multiple of 4.
Unfortunately I couldn't find a protocol specification of the xselinux
extension, to see whether the multiple of 4 is mandated by the protocol spec
or whether there should be align-pads. (and whether there should
be an align pad between "object_context" and "data_context"
Does anybody know where the spec for the xselinux protocol is?
There ought to be a spec somewhere for this because this is
security relevant.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
This pad is defined in the spec:
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml?id=xproto-7.0.28#n534
The server adds this pad in function GetHosts at:
http://cgit.freedesktop.org/xorg/xserver/tree/os/access.c?id=xorg-server-1.17.99.901#n1426
Function GetHosts is called in function ProcListHosts at:
http://cgit.freedesktop.org/xorg/xserver/tree/dix/dispatch.c?id=xorg-server-1.17.99.901#n3166
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
This pad is defined in the spec:
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml?id=xproto-7.0.28#n875
And it is also implemented in the Server:
http://cgit.freedesktop.org/xorg/xserver/tree/dix/dispatch.c?id=xorg-server-1.17.99.901#n532
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
Add explicit padding for:
* the struct "PRINTER":
- between list "name" and field "descLen"
I cannot read any of the formats that the xproto-spec is provided in.
(the postscript file is broken)
Therefore I provide the Xlib implementation as a reference:
The list "name" is read with _XReadPad or _XEatData:
http://cgit.freedesktop.org/xorg/lib/libXp/tree/src/XpPrinter.c?id=libXp-1.0.3#n158
_XReadPad does 4-byte padding:
http://cgit.freedesktop.org/xorg/lib/libX11/tree/src/xcb_io.c?id=libX11-1.6.3#n743
_XEatData does not do 4-byte padding:
http://cgit.freedesktop.org/xorg/lib/libX11/tree/src/xcb_io.c?id=libX11-1.6.3#n755
Since _XEatData is only a fallback when malloc fails, this is usually not
used. Therefore, using _XEatData as an alternative to _XReadPad in libXp
is a bug there.
- at the end of the struct "PRINTER"
this is OK because of the same reasons as above:
http://cgit.freedesktop.org/xorg/lib/libXp/tree/src/XpPrinter.c?id=libXp-1.0.3#n181
* the reply of request "PrintInputSelected"
- add a 1-byte pad at the start.
This is needed because all replies start with a 1-byte field that occupies
a place in the reply-header.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
Add explicit align-padding for struct SYSTEMCOUNTER:
Added 4-byte align-pad at the end.
This is field "p" in the spec of SYSTEMCOUNTER at:
http://cgit.freedesktop.org/xorg/proto/xextproto/tree/specs/sync.xml?id=xextproto-7.3.0#n983
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
Add explicit align-padding for:
* the request "SetPictureFilter":
Added 4-byte align-pad between list "filter" and list "values"
The spec does not show the padding:
http://cgit.freedesktop.org/xorg/proto/renderproto/tree/renderproto.txt?id=renderproto-0.11.1#n721
But there has to be padding to make accesses to 32-bit values in list
"values" aligned.
The Xlib implementation at
http://cgit.freedesktop.org/xorg/lib/libXrender/tree/src/Filter.c?id=libXrender-0.9.9#n160
correctly does the padding:
The string "filter" is written with "Data" which does 4-byte padding at the end:
http://cgit.freedesktop.org/xorg/lib/libX11/tree/include/X11/Xlibint.h?id=libX11-1.6.3#n535
http://cgit.freedesktop.org/xorg/lib/libX11/tree/src/xcb_io.c?id=libX11-1.6.3#n484
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
Add explicit align-padding for:
* the request "SetCrtcTransform"
Added 4-byte align-pad between list "filter_name" and list "filter_params"
The spec does not show the padding
http://cgit.freedesktop.org/xorg/proto/randrproto/tree/randrproto.txt?id=randrproto-1.5.0#n1173
But there has to be padding to make accesses to 32-bit values in list
"filter_params" aligned.
The Xlib implementation at
http://cgit.freedesktop.org/xorg/lib/libXrandr/tree/src/XrrCrtc.c?id=libXrandr-1.5.0#n327
correctly does the padding:
The string "filter" is written with "Data" which does 4-byte padding at the end:
http://cgit.freedesktop.org/xorg/lib/libX11/tree/include/X11/Xlibint.h?id=libX11-1.6.3#n535
http://cgit.freedesktop.org/xorg/lib/libX11/tree/src/xcb_io.c?id=libX11-1.6.3#n484
* the reply of request "GetCrtcTransform"
Added two 4-byte align-pads:
- between lists "pending_filter_name" and "pending_params"
the spec does not show the padding
http://cgit.freedesktop.org/xorg/proto/randrproto/tree/randrproto.txt?id=randrproto-1.5.0#n1205
But there has to be padding to make accesses to 32-bit values in list
"filter_params" aligned.
The Xlib implementation at
http://cgit.freedesktop.org/xorg/lib/libXrandr/tree/src/XrrCrtc.c?id=libXrandr-1.5.0#n434
adds the padding.
- between lists "current_filter_name" and "current_params"
the spec does not show the padding
http://cgit.freedesktop.org/xorg/proto/randrproto/tree/randrproto.txt?id=randrproto-1.5.0#n1205
But there has to be padding to make accesses to 32-bit values in list
"current_params" aligned.
The Xlib implementation at
http://cgit.freedesktop.org/xorg/lib/libXrandr/tree/src/XrrCrtc.c?id=libXrandr-1.5.0#n445
adds the padding.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
Signed-off-by: Robert Ancell <robert.ancell@canonical.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
Reviewed-by: Peter Harris <pharris@opentext.com>
|
|
Items need to be 4-aligned, see also 055c13096d94953de9ca62017f917572bd0e3f56
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
for function xcb_present_redirect_notify_sizeof
removing the explicit length of the list that was added since the
last release with the following changeset:
http://cgit.freedesktop.org/xcb/proto/commit/?id=c9b1523b23af52087a6354730f86b8d19fa3c7c0
Removing the explicit length again is OK because
implicit length is allowed when a list is the last
field of a request or event.
The compile problem that the prior change has tried to fix,
has been fixed in another way. Therefore removing the explicit length
is safe now.
Signed-off-by: Christian Linhart <chris@demorecorder.com>
|
|
Generated XCB code based on xprint.xml won't compile because the
arguments do not match now that the accessor functions for requests are
generated in libxcb.
Fix the definition so that the arguments match and the generated C code
for XPrint can compile.
Note, XPrint support has been removed in the X server since 2008, so
this is mostly cosmetic or even pedantic because XPrint is long gone.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
It was removed in:
http://cgit.freedesktop.org/xcb/proto/commit/?id=854d08c8242e8fbe7b5a000b75ec6845419806c4
Signed-off-by: Ran Benita <ran234@gmail.com>
Reviewed-by: Peter Harris <pharris@opentext.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
Signed-off-by: Ran Benita <ran234@gmail.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
Removed all the valueparam occurences from parsers and xml schema as
well along with all the protocol
definitions as valueparam has been replaced by switch bit-case
Signed-off-by: Jaya Tiwari <tiwari.jaya18@gmail.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
Changed valueparam to switch bitcase in Screensaver for the request
ScreenSaverSetAttributes
The changes of valueparam to switch has been made as per the specs for
the extension for the possible values of value-mask and value-list
ScreenSaverSetAttributes:
http://cgit.freedesktop.org/xorg/proto/scrnsaverproto/tree/specs/saver.xml#n633
Signed-off-by: Jaya Tiwari <tiwari.jaya18@gmail.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
Hi Chris, Vincent,
Thankyou for the comments.
I have rearranged the pad position in the patch below.
CreateWindow
ChangeWindowAttributes
ConfigureWindow
CreateGC
ChangeGC
ChangeKeyboardControl
The changes of valueparam to switch has been made as per the specs for
the extension for the possible values of value-mask and value-list
CreateWindow:
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml#n979
ChangeWindowAttributes:
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml#n1006
ConfigureWindow:
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml#n1105
CreateGC:
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml#n1815
ChangeGC:
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml#n1909
ChangeKeyboardControl:
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml#n2547
Casted KEYCODE32 as a CARD32 and added BOOL32 here instead of glx due
common to usage in other extensions.
Signed-off-by: Jaya Tiwari <tiwari.jaya18@gmail.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
Tested-by: Jaya Tiwari <tiwari.jaya18@gmail.com>
Tested-by: Christian Linhart <chris@demorecorder.com>
Comments by the reviewer Christian Linhart:
This will be API and ABI compatible for the same reasons as
your patch for the Render-extension.
I have checked this against the spec and implementation
and have not found any bugs.
Here are some explanations on how to overcome some difficulties
on reading the spec and the implementation.
The protocol specification is in some way misleading
and self-contradicting, but this can be resolved by
looking at the implementation.
Here are some problems with the spec:
The size of the whole value list is correctly described in
http://cgit.freedesktop.org/xorg/proto/xproto/tree/specs/encoding.xml?id=xproto-7.0.26
as
4n LISTofVALUE value-list
i.e. 4 bytes per value, where n is the number of bits set in the mask.
But some of the values themselves are describe incorrectly,
e.g. BITGRAVITY is described as 1 byte value.
This is not correct for two reasons:
* it contradicts LISTofVALUE ( except if implicit padding to 4 byte boundary after each value is assumed )
* even if implicit 4-byte padding is assumed, this contradicts the Xlib implementation in
http://cgit.freedesktop.org/xorg/lib/libX11/tree/src/Window.c?id=libX11-1.6.2#n55
where bit_gravity is assigned to an value of type "unsigned long", which is a 32-bit value
that's then sent verbatim to the server.
Depending on byte-order this is not the same as a 1-byte BOOL and a 3 byte padding.
* The server also treats BITGRAVITY as a 32-bit value.
There, all values from the value list are treated as values of type XID,
which is also 32 bit.
The protocol data is cast to an array of XID.
http://cgit.freedesktop.org/xorg/xserver/tree/dix/dispatch.c?id=xorg-server-1.16.3#n648
Later, a XID* is first dereferenced and then cast to an 8-bit type ( CARD8 ).
http://cgit.freedesktop.org/xorg/xserver/tree/dix/window.c?id=xorg-server-1.16.3#n1195
Please not that it is first dereferenced. Therefore a 32-bit value is retrieved from the given data.
Only after that it is cast to 8-bit, so that the least-significant 8 bits are taken.
That's not the same a 1 byte value plus 3 byte padding depending on byteorder.
Maybe the spec should be changed, so that all values of the valuelist are
described as 4 byte?
***
Thank you for writing this testcase.
I have checked this with respect to ABI and API compatibility and the result was the same for me, too.
I.e. this confirms that your patches for xproto are ABI and API compatible. ( as far as a testcase can show correctness of course... )
So, we can add Tested-by: headers with your and my name to your changes for "xproto", when merging them upstream.
Your changes for "render" were tested by Asalle and me, so we can add appropriate Tested-by: headers there, too.
The other changes follow the same pattern and are reviewed, so we probably do not need to test them separately.
Thank you for the testcase and your patches.
|
|
Replaced valueparam with a simple 32 bit field for PrintSelectInput
and PrintInputSelected by looking at the
lib code at :
http://cgit.freedesktop.org/xorg/lib/libXp/tree/src/XpInput.c?id=libXp-1.0.2
Signed-off-by: Jaya Tiwari <tiwari.jaya18@gmail.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
Review-comments by Christian Linhart:
This patch fixes a bug:
The prior use of valueparam was incorrect, and you have fixed that bug with this patch.
This can be verified by looking at the code which you have linked to.
and by looking at the definitions of the protocol structs:
For the PrintSelectInput-request:
http://cgit.freedesktop.org/xorg/proto/printproto/tree/Printstr.h?id=printproto-1.0.5#n439
For the reply of PrintInputSelected:
http://cgit.freedesktop.org/xorg/proto/printproto/tree/Printstr.h?id=printproto-1.0.5#n457
|
|
Changed valueparam to switch bitcase in render extension for the requests :
CreatePicture
ChangePicture
The changes of valueparam to switch has been made as per the specs for
the render extension for the possible
values of value-mask and value-list
CreatePicture :
http://cgit.freedesktop.org/xorg/proto/renderproto/tree/renderproto.txt?id=renderproto-0.11.1#n572
ChangePicture:
http://cgit.freedesktop.org/xorg/proto/renderproto/tree/renderproto.txt?id=renderproto-0.11.1#n668
The backward compatibility for the extension was checked successfully
by rendering shapes with libraries having
valueparam and then by having libraries with switch bitcase.
Signed-off-by: Jaya Tiwari <tiwari.jaya18@gmail.com>
Reviewed-by: Christian Linhart <chris@demorecorder.com>
|
|
branch demorec/ParametrizedStruct-V5
|
|
The current version of fixesproto.txt has cursor-name before
cursor-image, but all known implementations put cursor-name after
cursor-image on the wire.
Reviewed-by: Christian Linhart <chris@demorecorder.com>
Signed-off-by: Peter Harris <pharris@opentext.com>
|
|
Message-ID: <1410136150-30254-5-git-send-email-chris@demorecorder.com>
Patch-Thread-Subject: [Xcb] parametrized structs implemented
Patch-Set: ParametrizedStruct
Patch-Number: proto 5/5
Patch-Version: V1
Signed-off-by: Christian Linhart <chris@DemoRecorder.com>
|
|
complete definition of reply GetDeviceMotionEvents and struct DeviceTimeCoord
using paramref.
spec:
http://cgit.freedesktop.org/xorg/proto/inputproto/tree/specs/XIproto.txt?id=inputproto-2.3.1#n912
http://cgit.freedesktop.org/xorg/lib/libXi/tree/specs/encoding.xml?id=libXi-1.7.4#n983
code:
http://cgit.freedesktop.org/xorg/proto/inputproto/tree/XIproto.h?id=inputproto-2.3.1#n461
Message-ID: <1410136150-30254-4-git-send-email-chris@demorecorder.com>
Patch-Thread-Subject: [Xcb] parametrized structs implemented
Patch-Set: ParametrizedStruct
Patch-Number: proto 4/5
Patch-Version: V1
Signed-off-by: Christian Linhart <chris@DemoRecorder.com>
|
|
The first paragraph of the description is loosely based on
a description proposed by Ran Benita.
Message-ID: <1410136150-30254-3-git-send-email-chris@demorecorder.com>
Patch-Thread-Subject: [Xcb] parametrized structs implemented
Patch-Set: ParametrizedStruct
Patch-Number: proto 3/5
Patch-Version: V1
Signed-off-by: Christian Linhart <chris@DemoRecorder.com>
|
|
Message-ID: <1410136150-30254-2-git-send-email-chris@demorecorder.com>
Patch-Thread-Subject: [Xcb] parametrized structs implemented
Patch-Set: ParametrizedStruct
Patch-Number: proto 2/5
Patch-Version: V1
Signed-off-by: Christian Linhart <chris@DemoRecorder.com>
|
