Add PR_INT64 and PR_UINT64 macros for portable 64-bit integer constants

RESOLVED FIXED in 4.9

Status

enhancement
P3
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: pka, Assigned: pka)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Posted patch PR_UINT64, PR_INT64 macros (obsolete) — Splinter Review
NSPR provides macro for portable 64-bit signed int literals: LL_INIT, but there is no macro for unsigned ints.

SHA-512 uses it's own home-grown version of such macro: ULLC. GOST-28147 also needs PR_UINT64.

In addition, LL_* family may be inconvenient in that, it abstracts/hides underlying type: [struct{int32,int32}/int64], though application may want to be sure it uses real 64-bit computations.

The fix is to define PR_UINT64 if and only if the platform has 64-bit ints.

Proposed patch defines portable PR_UINT64 and it's complement, PR_INT64.
Attachment #529109 - Flags: review?(wtc)
Priority: -- → P3
Target Milestone: 4.8.8 → 4.8.9
Comment on attachment 529109 [details] [diff] [review]
PR_UINT64, PR_INT64 macros

r=wtc.  This should be fine.  Please document that the PR_INT64
and PR_UINT64 macros can only be used if HAVE_LONG_LONG is defined,
otherwise the LL_INIT macro defined in prlong.h has to be used.
(Today HAVE_LONG_LONG is defined for all the supported compilers.)

Nit: no need to right-adjust the signed and unsigned suffixes like
that:

+#define PR_INT64(x)  x ##  L
+#define PR_UINT64(x) x ## UL

Use just one space between "##" and the signed suffix.
Attachment #529109 - Flags: review?(wtc) → review+
Posted patch PR_UINT64, PR_INT64 macros (obsolete) — Splinter Review
According comment 1: documented; space between "##" and suffix deleted.
Attachment #529109 - Attachment is obsolete: true
Attachment #548403 - Flags: review?(wtc)
By the way, the comment before definition PR{,U}int64:

 /*
  * On 64-bit Mac OS X, uint64 needs to be defined as unsigned long long to
  * match uint64_t, otherwise our uint64 typedef conflicts with the uint64
  * typedef in cssmconfig.h, which CoreServices.h includes indirectly.
  */
  
[http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/include/prtypes.h&rev=3.44&mark=342-347#330]

seems to be invalid, because 

  1) handling of {,u}intNN types should be done in obsolete/protypes.h, not in prtypes.h

  2) the goal, claimed in the comment, is not achieved by the following code.
Posted patch PR_UINT64, PR_INT64 macros v3 (obsolete) — Splinter Review
Thank you for the patch.  I edited the comments in your patch.
It would be nice to add a test program or modify an existing
test program to use the new macros.
Attachment #548403 - Attachment is obsolete: true
Attachment #548403 - Flags: review?(wtc)
Target Milestone: 4.8.9 → 4.9
Patch checked in on the NSPR trunk (NSPR 4.9).

Checking in prtypes.h;
/cvsroot/mozilla/nsprpub/pr/include/prtypes.h,v  <--  prtypes.h
new revision: 3.46; previous revision: 3.45
done

Still need a patch to actually used the PR_INT64 or PR_UINT64
macros in NSPR itself or a test program.
Attachment #549018 - Attachment is obsolete: true
Assignee: wtc → andreev
Status: NEW → ASSIGNED
Summary: PR_UINT64 macro for portable 64-bit unsigned int literals → Add PR_INT64 and PR_UINT64 macros for portable 64-bit integer constants
This serves as a simple test for the new PR_INT64 macro.
Ideally we should modify the lltest.c test program to test
the PR_INT64 and PR_UINT64 macros (replace some of the LL_INIT
macros with PR_INT64 or PR_UINT64 macros), but I didn't have
time to do that today.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.