search

libsdl-org / sdl12-compat

An SDL-1.2 compatibility layer that uses SDL 2.0 behind the scenes.
Other
191 stars 40 forks source link

include: Use `struct private_hwdata` incomplete type for API compat #300

Closed smcv closed 1 year ago

smcv commented 1 year ago

Classic SDL 1.2 headers traditionally had this member as a pointer to the incomplete type struct private_hwdata. For whatever reason (presumably a sense of completeness) the SDLmm C++ binding[1] that is bundled in Debian package 'asc' (Advanced Strategic Command) defines an accessor for it, which is declared as returning struct private_hwdata * and has the obvious trivial implementation[2].

Implicit conversion from void * to struct private_hwdata * is allowed in C, but not in C++, so sdl12-classic's simplification of changing this member from struct private_hwdata * to void * breaks the build for SDLmm.

Arguably it's a bug that SDLmm has this accessor, because this member is for internal use by SDL and its type has no public definition, so there's no legitimate reason for third-party code to call the accessor; but it does exist, so 100% source compatibility with classic SDL 1.2 would require it to have the same type it historically had.

[1] https://sourceforge.net/projects/sdlmm/
[2] https://sourceforge.net/p/sdlmm/code/HEAD/tree/trunk/SDLmm/src/sdlmm_basesurface.h#l141

Resolves: https://github.com/libsdl-org/sdl12-compat/issues/299

If the sdl12-compat maintainers prefer to reject this PR as "SDLmm is wrong, sdl12-compat is already fine" then the change to fix compilation of asc should be as simple as deleting the hwdata accessor, which asc never calls anyway.

smcv commented 1 year ago

If the sdl12-compat maintainers prefer to reject this PR as "SDLmm is wrong, sdl12-compat is already fine" then the change to fix compilation of asc should be as simple as deleting the hwdata accessor, which asc never calls anyway.

Confirmed: deleting the accessor is enough to make SDLmm and asc compile successfully against sdl12-compat headers.

slouken commented 1 year ago

There's no reason not to have complete source compatibility with SDL 1.2. I'll go ahead and merge this. Thanks!