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

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

Open
prrace opened this issue Oct 17, 2019 · 5 comments
Open
Assignees

Comments

@prrace
Copy link

prrace commented Oct 17, 2019

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
Copy link
Author

prrace commented Oct 17, 2019

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.

@oddhack
Copy link
Collaborator

oddhack commented Nov 3, 2019

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.

@prrace
Copy link
Author

prrace commented Nov 3, 2019

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.

@oddhack
Copy link
Collaborator

oddhack commented Nov 3, 2019

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
Copy link
Collaborator

oddhack commented Nov 18, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
@oddhack @pdaniell-nv @prrace and others