Is there an x86 operating system where this is an issue?  Aside from the first member, do they overlap correctly?  If not, can you show the offset of each member for both structures?

[Replying by mail didn't work; using the web interface again now]

I don't have access to the Solaris machine where I ran into the bug, at the
moment, so I'll try to reconstruct what was going on from memory.

I have not encountered any problems with this on x86 systems, but I
suspect that this will go wrong on any system where 'sizeof(int) == 4'
and 'sizeof(int *) == 8'.

It can also be dependant on the compiler and on the optimisation flags
used, as padding is left unspecified in the C standard.

I think this is what happened for me:

                                       typedef struct {
                                           /* 0*/ unsigned int    dotclock;
                                           /* 1*/
                                           /* 2*/
                                           /* 3*/
typedef struct {
     /* 0*/ unsigned short  hdisplay;      /* 4*/ unsigned short  hdisplay;
     /* 1*/ ...                            /* 5*/ ...
     /* 2*/ unsigned short  hsyncstart;    /* 6*/ unsigned short  hsyncstart;
     /* 3*/ ...                            /* 7*/ ...
     /* 4*/ unsigned short  hsyncend;      /* 8*/ unsigned short  hsyncend;
     /* 5*/ ...                            /* 9*/ ...
     /* 6*/ unsigned short  htotal;        /*10*/ unsigned short  htotal;
     /* 7*/ ...                            /*11*/ ...
     /* 8*/ unsigned short  hskew;         /*12*/ unsigned short  hskew;
     /* 9*/ ...                            /*13*/ ...
     /*10*/ unsigned short  vdisplay;      /*14*/ unsigned short  vdisplay;
     /*11*/ ...                            /*15*/ ...
     /*12*/ unsigned short  vsyncstart;    /*16*/ unsigned short  vsyncstart;
     /*13*/ ...                            /*17*/ ...
     /*14*/ unsigned short  vsyncend;      /*18*/ unsigned short  vsyncend;
     /*15*/ ...                            /*19*/ ...
     /*16*/ unsigned short  vtotal;        /*20*/ unsigned short  vtotal;
     /*17*/ ...                            /*21*/ ...
     /*18*/ // Padding                     /*22*/ // Padding
     /*19*/ // Padding                     /*23*/ // Padding
     /*20*/ unsigned int    flags;         /*24*/ unsigned int    flags;
     /*21*/ ...                            /*25*/ ...
     /*22*/ ...                            /*26*/ ...
     /*23*/ ...                            /*27*/ ...
     /*24*/ int         privsize;          /*28*/ int         privsize;
     /*25*/ ...                            /*29*/ ...
     /*26*/ ...                            /*30*/ ...
     /*27*/ ...                            /*31*/ ...
     /*28*/ // Padding                     /*32*/ INT32       *private;
     /*29*/ // Padding                     /*33*/ ...
     /*30*/ // Padding                     /*34*/ ...
     /*31*/ // Padding                     /*35*/ ...
     /*32*/ INT32       *private;          /*36*/ ...
     /*33*/ ...                            /*37*/ ...
     /*34*/ ...                            /*38*/ ...
     /*35*/ ...                            /*39*/ ...
     /*36*/ ...                        } SDL_NAME(XF86VidModeModeInfo);
     /*37*/ ...
     /*38*/ ...
     /*39*/ ...
} SDL_NAME(XF86VidModeModeLine);

Padding is introduced to align ints on 4 bytes, and pointers on 8 bytes.
Everything lines up, up to the 'private' field, and then it goes wrong.
As the pointer is not aligned on the word size, dereferencing it will result
in a SIGBUS on systems where this is required (and this requirement even
exists on the x86).
On other systems, the pointer will probably be invalid, resulting in a
SIGSEGV.

Moving the dotclock field to the bottom of the SDL_XF86VidModeModeInfo
structure (and changing functions like SDL_XF86VidModeGetModeInfo which
do pointer tricks on it) would probably solve the problem.

There is more wrong with the code as it is now, btw. To get from
XF86VidModeModeInfo to XF86VidModeModeLine, the code does something like
this:
 	(SDL_NAME(XF86VidModeModeLine)*) ((char*)info + sizeof info->dotclock)
which assumes that there exists no padding after 'dotclock'. This happens to
be correct, but if the next field is ever changed to a 64-bits pointer,
this assumption will be wrong. Better would be:
 	(SDL_NAME(XF86VidModeModeLine)*) &info->hdisplay

But the cleanest solution would be to make XF86VidModeModeInfo
 	struct {
 		unsigned int dotclock;
 		struct SDL_NAME(XF86VidModeModeLine) modeLine;
 	} SDL_NAME(XF86VidModeModeInfo);

That way, the padding would be guaranteed by the C standard to be correct,
whatever platform SDL will be ported to later, and whatever fields are added
to these structures in the future, and there would be no need for pointer
tricks.
Unfortunately, anonymous structures aren't in the C standard, so you'd
have 'info.modeLine->...' every time when accessing the fields of
SDL_XF86VidModeModeInfo that are shared with SDL_XF86VidModeModeLine.
(gcc can do it with -fms-extensions btw)

Tagging this bug with "target-1.2.14" so we can try to resolve it for SDL 1.2.14.

Please note that we may choose to resolve it as WONTFIX. This tag is largely so we have a comprehensive wishlist of bugs to examine for 1.2.14 (and so we can close bugs that we'll never fix, rather than have them live forever in Bugzilla).

--ryan.

I'm hesitant to poke around in that code without a known broken test case.  If you find anywhere where the existing code doesn't work, please reopen this bug and let me know.

Thanks!

I seem to recall having hit this problem before, and thought we resolved it?

If we haven't, this should remain WONTFIX for 1.2. For 1.3, the solution is simply to use the system headers instead of our own copy, but I don't think we should change this for 1.2.

--ryan.