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 drkirkby)

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 drkirkby 10 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 10 years ago by drkirkby

  • Description modified (diff)

comment:2 Changed 10 years ago by drkirkby

  • Cc jsp jhpalmieri added
  • Description modified (diff)

comment:3 Changed 10 years ago by drkirkby

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 10 years ago by drkirkby

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

comment:4 Changed 10 years ago by drkirkby

  • Status changed from new to needs_review

comment:5 Changed 10 years ago by mariah

  • Status changed from needs_review to positive_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 10 years ago by drkirkby

  • Authors set to David Kirkby

comment:7 Changed 10 years ago by mariah

  • Reviewers set to Mariah Lenox

comment:8 Changed 10 years ago by rlm

  • Merged in set to sage-4.5.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.