[Xcb] [PATCH proto 1/1] Replace valueparam with switch-bitcase in Xproto

[Vincent Chen]

Hi,

I think there might be an error with the specification of
ConfigureWindow. According to:

http://www.x.org/releases/X11R7.7/doc/xproto/x11protocol.html#requests:ConfigureWindow

there should be a two-byte padding between the mask and the values.
The old valueparam with mask type of CARD16 (as opposed to the usual
CARD32) did not make this need for extra padding explicit. With the
new switch bitcase, however, the extra padding should be manually
inserted.

On Mon, Dec 22, 2014 at 10:35 AM, Jaya Tiwari <tiwari.jaya18 at gmail.com> wrote:
> @@ -1689,11 +1816,40 @@ <enum name="StackMode">
>    <request name="ConfigureWindow" opcode="12">
>      <pad bytes="1" />
>      <field type="WINDOW" name="window" />
> -    <field type="CARD16" name="value_mask" />
>      <pad bytes="2" />
> -    <valueparam value-mask-type="CARD16"
> -                value-mask-name="value_mask"
> -                value-list-name="value_list" />
> +    <field type="CARD16" name="value_mask" mask="ConfigWindow" />

Should add padding here:

<pad bytes="2" />

> +    <switch name="value_list">
> +        <fieldref>value_mask</fieldref>
> +        <bitcase>
> +          <enumref ref="ConfigWindow">X</enumref>
> +          <field type="INT32" name="x" />
> +        </bitcase>
> +        <bitcase>
> +          <enumref ref="ConfigWindow">Y</enumref>
> +          <field type="INT32" name="y" />
> +        </bitcase>
> +        <bitcase>
> +          <enumref ref="ConfigWindow">Width</enumref>
> +          <field type="CARD32" name="width" />
> +        </bitcase>
> +        <bitcase>
> +          <enumref ref="ConfigWindow">Height</enumref>
> +          <field type="CARD32" name="height" />
> +        </bitcase>
> +        <bitcase>
> +          <enumref ref="ConfigWindow">BorderWidth</enumref>
> +          <field type="CARD32" name="border_width" />
> +        </bitcase>
> +        <bitcase>
> +          <enumref ref="ConfigWindow">Sibling</enumref>
> +          <field type="WINDOW" name="sibling" altenum="Window"/>
> +        </bitcase>
> +        <bitcase>
> +          <enumref ref="ConfigWindow">StackMode</enumref>
> +          <field type="CARD32" name="stack_mode" enum="StackMode"/>
> +        </bitcase>
> +    </switch>
> +
>      <doc>
>        <brief>Configures window attributes</brief>
>        <description><![CDATA[

By the way, ConfigureWindow is the only place (that I know of) where
the mask type is CARD16. Every other uses of valueparam uses mask type
of CARD32. Not sure why the protocol designer didn't just make this
CARD32 as well, since the mask plus the two padding uses up 4 bytes
anyway...

Regards,

Vincent Chen