[Xcb] [PATCH proto 5/5] make xkb pass the alignment checker
Christian Linhart
chris at DemoRecorder.com
Wed Nov 4 19:53:27 PST 2015
Hi Ran,
Thank you for looking at this patch.
On 2015-11-05 01:15, Ran Benita wrote:
> On Sun, Nov 01, 2015 at 06:26:34PM +0100, Christian Linhart wrote:
>> 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.
> I haven't looked at the code for the checker yet, but the results here
> look good. I have a question below though (hopefully you haven't
> answered it already in the other commit message).
>
> And one style issue: the file uses tabs everywhere, but the patch uses
> spaces.
Thank you for noticing that, and sorry for that.
This must have slipped through my final style check.
I'll update the patch.
>
>> Signed-off-by: Christian Linhart <chris at demorecorder.com>
>> ---
>> src/xkb.xml | 25 +++++++++++--------------
>> 1 file changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/xkb.xml b/src/xkb.xml
>> index ad52ea2..0c5fa3f 100644
>> --- a/src/xkb.xml
>> +++ b/src/xkb.xml
>> @@ -741,14 +741,15 @@ <struct name="Section">
>>
>> <struct name="Listing">
>> <field name="flags" type="CARD16" />
>> <field name="length" type="CARD16" />
>> <list name="string" type="STRING8">
>> <fieldref>length</fieldref>
>> </list>
>> + <pad align="2" />
>> </struct>
>>
>> <struct name="DeviceLedInfo">
>> <field name="ledClass" type="LedClassSpec" enum="LedClass" />
>> <field name="ledID" type="IDSpec" altenum="ID" />
>> <field name="namesPresent" type="CARD32" />
>> <field name="mapsPresent" type="CARD32" />
>> @@ -1439,14 +1440,15 @@ <request name="SetMap" opcode="9">
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">KeyActions</enumref>
>> <list name="actionsCount" type="CARD8">
>> <fieldref>nKeyActions</fieldref>
>> </list>
>> + <pad align="4" />
>> <list name="actions" type="Action">
>> <fieldref>totalActions</fieldref>
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">KeyBehaviors</enumref>
>> <list name="behaviors" type="SetBehavior">
>> @@ -1454,14 +1456,15 @@ <request name="SetMap" opcode="9">
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">VirtualMods</enumref>
>> <list name="vmods" type="CARD8">
>> <popcount><fieldref>virtualMods</fieldref></popcount>
>> </list>
>> + <pad align="4" />
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">ExplicitComponents</enumref>
>> <list name="explicit" type="SetExplicit">
>> <fieldref>totalKeyExplicit</fieldref>
>> </list>
> I wonder why this one (for example) doesn't need an align? `SetExplicit`
> is 2 bytes long. Is there still implicit padding here?
No, there is no implicit padding.
The start of the bitcase is already aligned because all prior bitcases
end on a 4-byte aligned address.
I guess that the checker complained about SetExplicit non-alignment
before I added the 4-byte align-pad at the end of the "VirtualMods" bitcase.
The protocol spec probably states that all bitcases have to start
at a 4-byte aligned address. Therefore the proper way to fix
non-alignment of the "explicit"-list is to fix the alignment at the end
of the other bitcases.
Chris
BTW, the align-fixes in XKB are just there to get it through the checker.
But XKB is a good testcase for the checker, due to the complexity of the XKB protocol.
Xkb really needs complete rework, as by my RFC post a while ago.
I simply did not have the time to split that xkb-work into pieces and cross-reference them with the spec.
That's a huge amount of work, but that's the investment needed for having everything reviewed,
which is the basis of open-source development.
So, maybe I can make time for that, or somebody volunteers to document my prior XKB RFC-patch :-)
>
> Ran
>
>> </bitcase>
>> @@ -1663,28 +1666,15 @@ <request name="GetNames" opcode="17">
>> <list name="nLevelsPerType" type="CARD8">
>> <!-- Xlib uses NTypes here -
>> the spec says nKTLevels is correct, but
>> it does not work in reality
>> <fieldref>nKTLevels</fieldref> -->
>> <fieldref>nTypes</fieldref>
>> </list>
>> - <list type="CARD8" name="alignment_pad">
>> - <op op="-">
>> - <op op="&">
>> - <op op="+">
>> - <fieldref>nTypes</fieldref>
>> - <value>3</value>
>> - </op>
>> - <unop op="~">
>> - <value>3</value>
>> - </unop>
>> - </op>
>> - <fieldref>nTypes</fieldref>
>> - </op>
>> - </list>
>> + <pad align="4" />
>> <list name="ktLevelNames" type="ATOM">
>> <sumof ref="nLevelsPerType" />
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="NameDetail">IndicatorNames</enumref>
>> <list name="indicatorNames" type="ATOM">
>> @@ -1780,14 +1770,15 @@ <request name="SetNames" opcode="18">
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="NameDetail">KTLevelNames</enumref>
>> <list name="nLevelsPerType" type="CARD8">
>> <fieldref>nTypes</fieldref>
>> </list>
>> + <pad align="4"/>
>> <list name="ktLevelNames" type="ATOM">
>> <sumof ref="nLevelsPerType" />
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="NameDetail">IndicatorNames</enumref>
>> <list name="indicatorNames" type="ATOM">
>> @@ -2088,14 +2079,15 @@ <request name="GetKbdByName" opcode="23">
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">KeyActions</enumref>
>> <list name="acts_rtrn_count" type="CARD8">
>> <fieldref>nKeyActions</fieldref>
>> </list>
>> + <pad align="4" />
>> <list name="acts_rtrn_acts" type="Action">
>> <fieldref>totalActions</fieldref>
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">KeyBehaviors</enumref>
>> <list name="behaviors_rtrn" type="SetBehavior">
>> @@ -2103,26 +2095,29 @@ <request name="GetKbdByName" opcode="23">
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">VirtualMods</enumref>
>> <list name="vmods_rtrn" type="CARD8" mask="ModMask">
>> <popcount><fieldref>virtualMods</fieldref></popcount>
>> </list>
>> + <pad align="4" />
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">ExplicitComponents</enumref>
>> <list name="explicit_rtrn" type="SetExplicit">
>> <fieldref>totalKeyExplicit</fieldref>
>> </list>
>> + <pad align="4" />
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">ModifierMap</enumref>
>> <list name="modmap_rtrn" type="KeyModMap">
>> <fieldref>totalModMapKeys</fieldref>
>> </list>
>> + <pad align="4" />
>> </bitcase>
>> <bitcase>
>> <enumref ref="MapPart">VirtualModMap</enumref>
>> <list name="vmodmap_rtrn" type="KeyVModMap">
>> <fieldref>totalVModMapKeys</fieldref>
>> </list>
>> </bitcase>
>> @@ -2222,14 +2217,15 @@ <request name="GetKbdByName" opcode="23">
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="NameDetail">KTLevelNames</enumref>
>> <list name="nLevelsPerType" type="CARD8">
>> <fieldref>nTypes</fieldref>
>> </list>
>> + <pad align="4" />
>> <list name="ktLevelNames" type="ATOM">
>> <sumof ref="nLevelsPerType" />
>> </list>
>> </bitcase>
>> <bitcase>
>> <enumref ref="NameDetail">IndicatorNames</enumref>
>> <list name="indicatorNames" type="ATOM">
>> @@ -2348,14 +2344,15 @@ <request name="GetDeviceInfo" opcode="24">
>> <field name="dfltLedFB" type="CARD16" altenum="ID" />
>> <pad bytes="2" />
>> <field name="devType" type="ATOM" />
>> <field name="nameLen" type="CARD16" />
>> <list name="name" type="STRING8">
>> <fieldref>nameLen</fieldref>
>> </list>
>> + <pad align="4" />
>> <list name="btnActions" type="Action">
>> <fieldref>nBtnsRtrn</fieldref>
>> </list>
>> <list name="leds" type="DeviceLedInfo">
>> <fieldref>nDeviceLedFBs</fieldref>
>> </list>
>> </reply>
>> --
>> 2.1.4
>>
>> _______________________________________________
>> Xcb mailing list
>> Xcb at lists.freedesktop.org
>>
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
More information about the Xcb
mailing list