Opened 10 years ago
Closed 10 years ago
#9399 closed defect (fixed)
Remove Sun-specific junk in rings/finite_rings/stdint.h
Reported by: | drkirkby | Owned by: | drkirkby |
---|---|---|---|
Priority: | major | Milestone: | sage-4.5 |
Component: | porting: Solaris | Keywords: | |
Cc: | jsp, jhpalmieri | Merged in: | sage-4.5.rc0 |
Authors: | David Kirkby | Reviewers: | Mariah Lenox |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The file sage-4.4.4.alpha1/sage/rings/finite_rings/stdint.h
has this in it:
#if defined(__sun) typedef int int_fast32_t; typedef long long int_fast64_t; #else #include <stdint.h> #endif #define INTEGER_MOD_INT32_LIMIT 46341 // = ceil(sqrt(2^31-1)) #define INTEGER_MOD_INT64_LIMIT 2147483647 // = 2^31-1 for now, should be 3037000500LL = ceil(sqrt(2^63-1))
I can only assume someone added this Sun-specific code for a very old version of Solaris. Any Solaris 10 release will include the header file <stdint.h>, so there is no need to have this.
The code as it stands conflicts with a Solaris header file, which defines int_fast64_t as being a 'long' and not a 'long long' in 64-bit mode. The code as show is only valid for 32-bit.
The following will save a few bytes, and will go further to advance the state of a 64-bit Solaris port.
#include <stdint.h> #define INTEGER_MOD_INT32_LIMIT 46341 // = ceil(sqrt(2^31-1)) #define INTEGER_MOD_INT64_LIMIT 2147483647 // = 2^31-1 for now, should be 3037000500LL = ceil(sqrt(2^63-1))
Attachments (1)
Change History (9)
comment:1 Changed 10 years ago by
- Description modified (diff)
comment:2 Changed 10 years ago by
- Cc jsp jhpalmieri added
- Description modified (diff)
comment:3 Changed 10 years ago by
Changed 10 years ago by
Ensure the header file is included on all systems, not excluding Solaris as before.
comment:4 Changed 10 years ago by
- Status changed from new to needs_review
comment:5 Changed 10 years ago by
- Status changed from needs_review to positive_review
- patch applied on skynet/taurus to sage-4.4.4.1
- ./sage -b done
- make testall
All test passed. Postive review!
comment:6 Changed 10 years ago by
comment:7 Changed 10 years ago by
- Reviewers set to Mariah Lenox
comment:8 Changed 10 years ago by
- Merged in set to sage-4.5.rc0
- Resolution set to fixed
- Status changed from positive_review to closed
With the attached patch, Sage builds on both 32-bit SPARC (like t2) in 32-bit and on OpenSolaris? as 64-bit. I can only assume this was added at some point in the past to attempt to build Sage on Solaris 9, which would not have this header file. But even Solaris 10 03/05 (the first verison of Solaris 10) has this header file
Dave