Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch to make SDL_config.h universal for 32/64 architectures. #904

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

Patch to make SDL_config.h universal for 32/64 architectures. #904

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Labels
enhancement New feature or request

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: Linux, All

Comments on the original bug report:

On 2013-06-05 03:17:35 +0000, Azamat H. Hackimov wrote:

Created attachment 1174
multiarch-enviroment.patch

Gentoo Linux on 64-bit system supports compiling and installing both versions of library - 32-bit and 64-bit (multilib installation). Only restriction of this feature files from include dir should be identical (i.e. platform independent).

Included patch contains two commits that makes SDL_config.h universal for both architectures.

On 2013-06-06 15:16:34 +0000, Ryan C. Gordon wrote:

I'm trying to understand the LONG64 #define...won't this make _XData32 and _XRead32 show up in the headers for 32-bit systems, where they don't exist?

I might be misunderstanding the intention here, though.

--ryan.

On 2013-06-08 06:38:47 +0000, Azamat H. Hackimov wrote:

#define LONG64 is required only for _XRead32 struct changes, it can be defined as -DLONG64. In 32-bit arch SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 may be to appear in SDL_config.h, but this affects only one piece of code that can be active only on 64-bit arch (#ifdef LONG64 in code), so SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 is harmless in 32bit.

In Gentoo we have follow structure of installed SDL2:

/usr/lib32 - 32bit libs
/usr/lib64 - 64bit libs
/usr/include - header files

One packet can install both versions - 32 and 64. Trouble is that currently generated SDL_config.h from /usr/include is differs for those targets.

On 2013-07-12 02:17:44 +0000, Sam Lantinga wrote:

I applied the SIZEOF_VOIDP patch. I don't completely understand the LONG64 _XData32 bit though.

On 2013-07-12 22:15:45 +0000, Ryan C. Gordon wrote:

(Sorry if you get a lot of copies of this email, we're touching dozens of bug reports right now.)

Tagging a bunch of bugs as target-2.0.0, Priority 2.

This means we're in the final stretch for an official SDL 2.0.0 release! These are the bugs we really want to fix before shipping if humanly possible.

That being said, we don't promise to fix them because of this tag, we just want to make sure we don't forget to deal with them before we bless a final 2.0.0 release, and generally be organized about what we're aiming to ship.

Hopefully you'll hear more about this bug soon. If you have more information (including "this got fixed at some point, nevermind"), we would love to have you come add more information to the bug report when you have a moment.

Thanks!
--ryan.

On 2013-07-16 13:15:47 +0000, Azamat H. Hackimov wrote:

Here difference between x86 and amd64 builds without my patch against libX11 < 1.5.99.902 (I'm use CMake and assume that for autotools output would be same):

diff -Nuar SDL-2.0.0-7338-x86/include/SDL_config.h SDL-2.0.0-7338-amd64/include/SDL_config.h
--- SDL-2.0.0-7338-x86/include/SDL_config.h 2013-07-16 22:58:38.086943712 +0600
+++ SDL-2.0.0-7338-amd64/include/SDL_config.h 2013-07-16 22:58:38.085943712 +0600
@@ -272,7 +272,7 @@
#define SDL_VIDEO_DRIVER_X11_XSHAPE 1
#define SDL_VIDEO_DRIVER_X11_XVIDMODE 1
#define SDL_VIDEO_DRIVER_X11_SUPPORTS_GENERIC_EVENTS 1
-#define SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 1
+/* #undef SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 */
#define SDL_VIDEO_DRIVER_X11_CONST_PARAM_XEXTADDDISPLAY 1
#define SDL_VIDEO_DRIVER_X11_HAS_XKBKEYCODETOKEYSYM 1

With my patch SDL_config.h would be same for both arches and SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 will be undefined.

Let's see code, where SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 is used (src/video/x11/SDL_x11sym.h):

#ifdef LONG64
SDL_X11_MODULE(IO_32BIT)
#if SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32
SDL_X11_SYM(int,_XData32,(Display *dpy,register _Xconst long *data,unsigned len),(dpy,data,len),return)
#else
SDL_X11_SYM(int,_XData32,(Display *dpy,register long *data,unsigned len),(dpy,data,len),return)
#endif
SDL_X11_SYM(void,_XRead32,(Display *dpy,register long *data,long len),(dpy,data,len),)
#endif

First at all, this code applies only for amd64 (LONG64 check), so even with "#define SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 1" in x86/SDL_config.h this code will be disabled. SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 is used only for one purpose - detect changes in API for libX11, which for amd64 and x86 is same.

In other words, no matter SDL_VIDEO_DRIVER_X11_CONST_PARAM_XDATA32 defined on x86 or not, it not applies to actual code. We can use anything value for it, so let's use same as amd64!

On 2013-08-03 03:17:10 +0000, Ryan C. Gordon wrote:

Still not sold on the LONG64 thing...seems dangerous to #define that and expect Xlib headers to always do the right thing now and in the future on all platforms. If this makes the compile fail on x86 at some point, we'll misconfigure.

As for SIZEOF_VOIDP ... Sam, can we just nuke that from configure/cmake? We don't actually use it anywhere in SDL, as far as I can tell.

--ryan.

On 2015-02-19 05:22:18 +0000, Ryan C. Gordon wrote:

Marking a large number of bugs with the "triage-2.0.4" keyword at once. Sorry if you got a lot of email from this. This is to help me sort through some bugs in regards to a 2.0.4 release. We may or may not fix this bug for 2.0.4, though!

On 2015-04-07 04:57:55 +0000, Ryan C. Gordon wrote:

(sorry if you get a lot of copies of this email, I'm marking several bugs at once)

Marking bugs for the (mostly) final 2.0.4 TODO list. This means we're hoping to resolve this bug before 2.0.4 ships if possible. In a perfect world, the open bug count with the target-2.0.4 keyword is zero when we ship.

(Note that closing a bug report as WONTFIX, INVALID or WORKSFORME might still happen.)

--ryan.

On 2015-06-08 05:17:08 +0000, Ryan C. Gordon wrote:

I took out the _XData32 test that caused the different in SDL_config.h in this commit: https://hg.libsdl.org/SDL/rev/d5955671f259

...so now the header should be consistent on both targets and I'm closing this bug.

--ryan.

@SDLBugzilla SDLBugzilla added the enhancement New feature or request label Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant