[Xcb] xb/proto portion of [PULL v3] XResource extension v1.2

[Peter Harris]

On Tue, Apr 24, 2012 at 6:35 AM, Erkki Seppala wrote:
>> I'm also unsure about the <struct name="ClientIdValue"> definition,
>> where the XResproto.h header says:
>>  CARD32  length B32;
>>  // followed by length CARD32s
>
>
> Hmm, this comment is actually wrong. According to resproto.txt it is
> followed by CARD32s, but length is in multiples of 4. In the case of PID its
> value is 4. I shall quote the passage (lines 127+ of resourceproto.txt):

That's not how I read the spec you quoted. My reading of the spec
fragment agrees with the comment.

> CLIENTIDVALUE [ spec:   CLIENTIDSPEC
>                length: CARD32
>                value:  LISTofCARD32 ]

Value is a list of CARD32.

...

> length
>    Specifies the length of an ID in units of CARD32. The length
>    depends on the ID type. In version 1.2 the lengths are 0 for
>    ClientXid and 4 for LocalClientPid. The length of ClientXid is 0
>    because that is already stored in the spec field.

"in units of CARD32" means the number of CARD32s expected to be in the
list (number of bytes is length * 4). Then it goes on to say that the
length is 4, which I understood to mean that the length field is 1 on
the wire (or that there are four CARD32s for a LocalClientPid, but
this interpretation disagrees with the description below).

If this isn't how it functions, it (either the server implementation
or the spec) needs to be fixed.

I was going to quote the core spec where it uses "units of CARD32" to
mean what I think it means, but every instance of "units of" was
followed or preceded by the phrase "four bytes".

> value
>    Actual ID data. In version 1.2 this is missing for ClientXid and
>    consists of a single CARD32 for LocalClientPid.

CARD32.

> Perhaps resourceproto.txt could be revised to state that value is
> LISTofCARD8, but how would the alignment requirements then be indicated?

The core spec uses
 p                                     unused, p=pad(n)

Extension specs in the style of resourceproto.txt leave the padding implicit.

I'm more concerned about the swapping. If the value is swapped on the
wire, XCB really wants to know.

>> the res.xml has CARD8's instead of CARD32's:
>>       <field type="CARD32" name="length" />
>>       <list type="CARD8" name="client_ids">
>>           <fieldref>length</fieldref>
>>       </list>
>>
>> CARD8 seems too short for a pid_t, but perhaps I'm misunderstanding the
>> xcb type handling?
>
>
> Which is what is stated here: the field actually contains opaque data

Can we fix that, please?

If it must be opaque, so be it, but I don't see anything in the part
of the spec that you quoted about non-CARD32 data here.

If you're trying to leave it open for non-CARD32 data in the future,
could you please add additional requests (in a future version of the
spec) instead? XCB already doesn't represent ChangeProperty very well.
See CompositeGlyphs(8|16|32) in the RENDER spec for an example of
protocol that is easier to represent in XCB.

http://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html#requests:ChangeProperty
vs
http://cgit.freedesktop.org/xorg/proto/renderproto/tree/renderproto.txt#n972

>, so a
> pid is stored in four octets. But perhaps it should be better written
> something like (if the previous specification was revised):
>
>  <list type="CARD32" name="client_ids">
>    <op>/</op>
>      <op>+</op>
>        <fieldref>length</fieldref>
>        <value>3</value>
>      </op>
        <value>4</value>
>    </op>
>  </list>
>
> or not, hopefully someone can chime in.

If it really is a list of CARD32s, with the length given in bytes
(unlike most of the rest of X11, which gives lengths in list units),
then this is a reasonable way to express that.

> It would be nice to express that the
> padding isn't part of the data. Or does it automatically get padded to 4
> bytes?

It will automatically get padded to the natural alignment of the next
thing on the wire. Since it is a list of CARD32s, there shouldn't be
any padding required.

Peter Harris