[Xcb] [PATCH] XCB-XML: Proposal for alignment padding

[Julien Cristau]

On Mon, Nov  8, 2010 at 19:39:36 +0300, Evgeny M. Zubok wrote:

> Julien Cristau <jcristau at debian.org>
> writes:
> 
> >> After examining a full set of X Protocol extensions which are
> >> available in XCB I conclude that the following form is suitable to
> >> cover different padding schemas:
> >> 
> >> <pad align="integer">expression</pad>
> >> 
> 
> > I don't get why you need the expression here.  What you seem to want is
> > 'the next field is aligned on a "integer" boundary', which doesn't
> > require a reference to the preceding field(s), unless I'm missing
> > something?
> 
> This is usually the case with the last pad in structure. The last pad
> expression in requests and replies can be omited by code generator and
> can be interpreted as 4-byte alignment from all previous
> fields. However, the X Window System specification defines padding as
> function pad(E), where E is expression.
> 
> Look at the example 3. If we compute padding only from preceding list
> 'string', we'll get incorrect pad size for structure SYSTEMCOUNTER
> (sync.xml):
> 
> 4    COUNTER    counter
> 8    INT64      resolution
> 2    n          length of name in bytes
> n    STRING8    name
> p               pad, p=pad(n+2)
> 
> If the 'name' would have length 1, then pad without expression will be 3
> bytes. It should be equal to pad(1+2)=1 byte. Although the same result
> we will get if compute padding from total size of structure (bacause
> first two fields already have a total size that multiple of 4). Below
> the one iteration step on the LISTofSYSTEMCOUNTERs from
> xorg/lib/libXext/src/XSync.c:
> 
> pNextWireSysCounter = (xSyncSystemCounter *)
>   (((char *)pWireSysCounter) + ((SIZEOF(xSyncSystemCounter) +
> 		     pWireSysCounter->name_length + 3) & ~3));
> 
> As we can see, the padding is calculated not from the size of list
> 'name' but from the sum of previous fields and the list size.
> 
I never said calculate padding from just the previous field.  I said
make it so that the next field is on a 4-byte (or 2-byte, in whatever
case that happens) boundary.

> If we will drop the expression from <pad> then we should constanly
> calculate current total legth of request and insert
> p=pad(total_length_at_this_place). IMHO, It's better to express padding
> as it is described in specification. Then we don't need constantly
> calculate the total length of all fields from structure and could only
> include those lengths which are referenced by expression inside
> <pad>. Anyway, the implementation details can be chosen by implementor
> of code generator.

Don't you need to know the size of the request anyway?  Also I'd rather
have that work done in the code generator than in the xml description,
to keep that as readable as possible (and incidentally, avoid
introducing new bugs there).  Maybe that's just me though.

Cheers,
Julien