Opened 13 years ago

Closed 13 years ago

#9399 closed defect (fixed)

Remove Sun-specific junk in rings/finite_rings/stdint.h

Reported by: David Kirkby Owned by: David Kirkby
Priority: major Milestone: sage-4.5
Component: porting: Solaris Keywords:
Cc: Jaap Spies, John Palmieri Merged in: sage-4.5.rc0
Authors: David Kirkby Reviewers: Mariah Lenox
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by David Kirkby)

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)

9399.patch (790 bytes) - added by David Kirkby 13 years ago.
Ensure the header file is included on all systems, not excluding Solaris as before.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 13 years ago by David Kirkby

Description: modified (diff)

comment:2 Changed 13 years ago by David Kirkby

Cc: Jaap Spies John Palmieri added
Description: modified (diff)

comment:3 Changed 13 years ago by David Kirkby

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

Changed 13 years ago by David Kirkby

Attachment: 9399.patch added

Ensure the header file is included on all systems, not excluding Solaris as before.

comment:4 Changed 13 years ago by David Kirkby

Status: newneeds_review

comment:5 Changed 13 years ago by Mariah Lennox

Status: needs_reviewpositive_review
  1. patch applied on skynet/taurus to sage-4.4.4.1
  2. ./sage -b done
  3. make testall

All test passed. Postive review!

comment:6 Changed 13 years ago by David Kirkby

Authors: David Kirkby

comment:7 Changed 13 years ago by Mariah Lennox

Reviewers: Mariah Lenox

comment:8 Changed 13 years ago by Robert Miller

Merged in: sage-4.5.rc0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.