Bug 45202

Summary: xf86vmproto.h xXF86VidModeModeInfo hskew B16
Product: xorg Reporter: Kevin Ryde <user42_kevin>
Component: Protocol/CoreAssignee: Jamey Sharp <jamey>
Status: RESOLVED MOVED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: high CC: jeremyhu
Version: unspecified   
Hardware: x86 (IA32)   
OS: All   
Whiteboard: 2012BRB_Reviewed
i915 platform: i915 features:

Description Kevin Ryde 2012-01-24 16:06:26 UTC
(Is component "Other" right for extension protocol stuff, or is it "Protocol/Core"?)

Nosing around xf86vmproto.h I saw the xXF86VidModeModeInfo struct has its hskew field as

    CARD32	hskew B16;

Is it supposed to be B32?  The B macros are empty except on Cray are they?  Perhaps it doesn't make a difference, or perhaps it lets the compiler bit field packing leak out ... I couldn't tell.

I wondered too if the "pad1" field there is supposed to be 32-bits.  gcc reports unexpressed alignment padding after "pad1" before "flags".

    gcc -Wpadded -x c -fsyntax-only /usr/include/X11/extensions/xf86vmproto.h
    =>
    /usr/include/X11/extensions/xf86vmproto.h:169:12: warning: padding struct to align 'flags' [-Wpadded]

Was hskew going to be 16 then became 32 and pad1 not updated, or something?
Comment 1 Jeremy Huddleston Sequoia 2012-06-12 03:31:24 UTC
Jamey, would you mind looking into this?
Comment 2 Adam Jackson 2018-06-12 19:09:37 UTC
Mass closure: This bug has been untouched for more than six years, and is not
obviously still valid. Please reopen this bug or file a new report if you continue to experience issues with current releases.
Comment 3 Kevin Ryde 2018-06-16 06:31:25 UTC
I'm confident CARD32 shouldn't be bitfield B16.  Compilers needing bitfield help like that are likely to be slowly reducing, but if the B are going to be there then they may as well be right.

The gcc invocation I reported still gives the same warning at the same line (debian i386 32-bit gcc 7.3).
Comment 4 Alan Coopersmith 2018-06-16 15:43:07 UTC
Compilers that use this are completely gone since Cray support was removed.
<X11/Xmd.h> now just defines them to nothing:
# define B32 /* bitfield not needed on architectures with native 32-bit type */
# define B16 /* bitfield not needed on architectures with native 16-bit type */

It does look like pad1 is a mistake though, and the compatible fix would be
declaring it 32-bit even though we wouldn't a 32-bit pad there if it was being
done from scratch today.
Comment 5 GitLab Migration User 2018-08-10 19:55:14 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/xorgproto/issues/3.

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.