<div dir="ltr">Hi,<div>I still think this is the correct approach, and want to finish up an implementation with testcases in xkbcommon to make sure it doesn't do anything surprising, as well as checking that old xkbcommon and xkbcomp still parse it right.</div>

<div><br><div class="gmail_extra"><div class="gmail_quote">On 9 January 2013 00:46, Peter Hutterer <span dir="ltr"><<a href="mailto:peter.hutterer@who-t.net" target="_blank">peter.hutterer@who-t.net</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><span style="color:rgb(34,34,34)">ok, I looked at all four and they do look good and look like they do what</span><br>

</div>
they should. The only two comments I have at this point are:<br>
* where are the bytes guaranteed to be zero? Not questioning, I just want to<br>
  double-check and it's outside the patch contexts<br>
* XkbModAction increases by 2 bytes. this is really my main worry here,<br>
  it is semi-public API. from a protocol POV it's fine because XKbAction<br>
  forces it to 8 bytes anyway so none of the protocol parsers should be<br>
  affected (fwiw, xcb would need an update here as well)<br>
<br>
So really, the only danger we have here is potentially breaking clients that<br>
expect XkbModAction to be 6 bytes, because of whatever reason. The safest<br>
approach would be to add a new struct here (XkbModAction2) and use that<br>
everywhere to avoid this issue. Daniel, any opinion?<br></blockquote><div><br></div><div>So yeah, we'd probably need to add an XkbModAction2 here, which wouldn't be an ABI break, so no problem.  However, XkbSetMap (and XkbSetCompatMap) need to know whether or not those last two bytes are going to be valid when they see XkbSA_LockMods, or potentially uninitialised garbage.  Similarly, both libX11 and its users are going to need to know when reading an XkbDescRec (and XkbCompatMapRec) whether those two bytes are valid.  But we can't assume that all users have been updated, so introducing a new action type for reading (e.g. XkbGetMap and friends) is out of the question.  Here's what I think would work:</div>

<div>  - bump the protocol minor version to 1.1, specifying that the extra bytes will be ignored on the wire for all earlier versions, but must be valid for 1.1;</div><div>  - bump the XKB library version in libX11 to 1.1;</div>
<div style>  - always return correct values (either 0 for nothing, or a valid value) for the new two bytes in actions with library version 1.1;</div><div style>  - accept new action values (e.g. XkbSA_SetMods2) in XkbSetMap and friends starting with 1.1, but noting that clients must explicitly check the version and not send these actions to 1.0 libraries.</div>
<div style><br></div><div style>Alternately:<br></div>
<div>  - forget it ever happened for XkbSetMap and XkbGetMap: don't report those extended bytes to clients, and ignore them on inbound requests;</div><div>  - introduce a new wire request which allows users to get and set the map via its textual representation, which also neatly gives us extended keycodes over the wire;</div>

<div>  - recommend clients use xkbcommon to parse the textual map.</div><div><br></div><div>Cheers,</div><div>Daniel </div></div></div></div></div>