Bug 67782 - xproto.xml: ConfigureWindow request with additional 'value_mask' field
Summary: xproto.xml: ConfigureWindow request with additional 'value_mask' field
Status: RESOLVED MOVED
Alias: None
Product: XCB
Classification: Unclassified
Component: Protocol (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: xcb mailing list dummy
QA Contact: xcb mailing list dummy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-05 10:53 UTC by Jochen Keil
Modified: 2019-02-16 19:39 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
xproto: Replace valueparam with list in ConfigureWindow (1.53 KB, patch)
2013-08-05 19:49 UTC, Daniel Martin
Details | Splinter Review

Description Jochen Keil 2013-08-05 10:53:03 UTC
Hello,

In the protocol description for ConfigureWindow there is an extra "value_mask" field. This field is not used in the generated xcb_configure_window functions.

Since there is already field called "value_mask" for the valueparam, I suspect this is not wanted. This also creates errors when trying to generate functions with two "value_mask" parameters.

  <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" />
Comment 1 Daniel Martin 2013-08-05 19:47:58 UTC
(In reply to comment #0)
> Hello,
> 
> In the protocol description for ConfigureWindow there is an extra
> "value_mask" field. This field is not used in the generated
> xcb_configure_window functions.
> 
> Since there is already field called "value_mask" for the valueparam, I
> suspect this is not wanted. This also creates errors when trying to generate
> functions with two "value_mask" parameters.
> 
>   <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" />

This has been done on purpose, see commit b684b0d:
    Re-fix the ConfigureWindow request padding issue.
    
    We rely on the fact that the valueparam field is treated as a list,
    and, like any other list, the Python code will check if the length
    field is previously defined in the structure before adding a new one.
    This allows us to insert the necessary 2-byte padding.


The best fix would be to replace it with a switch. But, a lot of projects won't be happy with that:
    http://codesearch.debian.net/search?q=xcb_configure_window


Though, the valueparam can been replaced by a list as in the following attachment without any drawbacks = without any changes in the generated code.

Replacing valueparams is a good thing, but replacing it with a list is not a win. So, I don't have any opinion what we should do with that field.
Comment 2 Daniel Martin 2013-08-05 19:49:13 UTC
Created attachment 83678 [details] [review]
xproto: Replace valueparam with list in ConfigureWindow
Comment 3 GitLab Migration User 2019-02-16 19:39:58 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/proto/xcbproto/issues/5.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.