OpenGL-Registry icon indicating copy to clipboard operation
OpenGL-Registry copied to clipboard

glxext.h re-defining int64_t on Solaris - causing compilation failure.

Open prrace opened this issue 6 years ago • 5 comments

glxext.h has this block of code below, which seems to pre-date the github migration. OpenJDK (http://hg.openjdk.java.net/jdk/jdk) has a truly ancient copy of this file and I am in the process of updating it, however when included in the JDK sources, it fails to compile on 64 bit Solaris 11 SPARC due to a typedef re-definition :

"glxext.h", line 690: error: typedef redeclared: int64_t (E_TYPEDEF_REDECLARED)

The line numbers don't match exactly but I've pointed below to where this occurs Also the indentation is mine to make things a little clearer. Solaris' inttypes.h includes sys/int_types.h which provides the definition. Then it gets re-defined by explicit code in glxext.h which presumably was to cater for the case where it isn't defined.

My best guess is that once upon a time Solaris did not define these 64 bit names and so the extra code for sun and digital was added. However this must have been a REALLY long time ago, so long ago that it doesn't add up since Solaris has been 64 bit since Solaris 7 in 1998. Since the O/S provides the header it can't matter which compiler is used. So far as I can tell from looking at the Solaris 11 header files, so long as _LP64 is defined, it will define these names. If that isn't defined I don't know if you'd want them anyway ...

Perhaps some combination of compiler option and versions would make it necessary but we only specify -xc99=all,no_lib to SS12 update 6. I think that explains why we don't go down the STDC_VERSION >= 199901 path but nothing else that looks like it would affect the checks here. I can fix it in my copy so it builds where I need it to build but it is better if someone upstream has an idea what it should look like to work on Solaris and not break elsewhere.

#ifndef GLEXT_64_TYPES_DEFINED
/* This code block is duplicated in glext.h, so must be protected */
#define GLEXT_64_TYPES_DEFINED
/* Define int32_t, int64_t, and uint64_t types for UST/MSC */
/* (as used in the GLX_OML_sync_control extension). */
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
  #include <inttypes.h>
#elif defined(__sun__) || defined(__digital__)
  #include <inttypes.h>                           <<<<< FIRST DEFINED
  #if defined(__STDC__)
    #if defined(__arch64__) || defined(_LP64)
      typedef long int int64_t;                   <<<<<< RE-DEFINITION HERE
      typedef unsigned long int uint64_t;
    #else
      typedef long long int int64_t;
      typedef unsigned long long int uint64_t;
    #endif /* __arch64__ */
  #endif /* __STDC__ */
#elif defined( __VMS ) || defined(__sgi)
  #include <inttypes.h>
#elif defined(__SCO__) || defined(__USLC__)
  #include <stdint.h>
#elif defined(__UNIXOS2__) || defined(__SOL64__)
  typedef long int int32_t;
  typedef long long int int64_t;
  typedef unsigned long long int uint64_t;
#elif defined(_WIN32) && defined(__GNUC__)
  #include <stdint.h>
#elif defined(_WIN32)
  typedef __int32 int32_t;
  typedef __int64 int64_t;
  typedef unsigned __int64 uint64_t;
#else
/* Fallback if nothing above works */
  #include <inttypes.h>
#endif

My suggested fix would make the sun case have only the include.

#elif defined(__sun__)
  #include <inttypes.h>
#elif defined(__digital__)
  #include <inttypes.h>
  #if defined(__STDC__)
    #if defined(__arch64__) || defined(_LP64)
      typedef long int int64_t;
      typedef unsigned long int uint64_t;
    #else
      typedef long long int int64_t;
      typedef unsigned long long int uint64_t;
    #endif /* __arch64__ */
  #endif /* __STDC__ */

prrace avatar Oct 17 '19 21:10 prrace

Oh, BTW this comment /* This code block is duplicated in glext.h, so must be protected */ must be out of date. I don't see anything like that there.

prrace avatar Oct 17 '19 21:10 prrace

Sorry for the delay responding. I'd like to do a sanity check with Mesa, since they're probably the only OpenGL project still supporting any kind of Solaris drivers, and if they're OK with it happy to make this change. Unfortunately freedesktop.org is down right now so I'm unsure how to submit the question to them, but will hopefully remember to check back soon. If you're a Mesa developer, maybe you could instead run this by the right people to get some sort of consensus from the project?

Also, could you submit a PR against the repository to make these changes? Note the boilerplate code is imbedded in glx.xml, from whence it is included in the generated glxext.h.

oddhack avatar Nov 03 '19 11:11 oddhack

Initially I noticed this with the version I pulled from Mesa, but I supposed the origin (here) was the place to report it. I am not associated with the mesa project.

Got it about the xml being the file to manually update.

prrace avatar Nov 03 '19 15:11 prrace

This is the canonical source, but since the combination of Solaris + GLX is only supported by Mesa, AFAIK, it seems prudent to check with them as I have no way to validate the PR myself. I'll try to check in with them soon - in process of creating a freedesktop.org account now.

oddhack avatar Nov 03 '19 15:11 oddhack

@prrace I never got a freedesktop.org account created - wouldn't send me the validation email AFAICT. Maybe you could follow up with them.

oddhack avatar Nov 18 '19 14:11 oddhack