when a native 64-bit int type is available, it would be nice if LL_ZERO would not need a function call. it can just be a #define for 0LL. similar for LL_MAXUINT etc.
15 years ago
Status: NEW → ASSIGNED
Thanks for the patch. I moved some code around so that we define the macros in the same order in all cases. The real change I made is to define LL_MININT as an expression. This is how the corresponding macro LLONG_MIN is defined on many platforms. I don't know why it's necessary though. (I did the same thing with PR_INT32_MIN in prtypes.h.)
Comment on attachment 170840 [details] [diff] [review] patch v1.1 looks good to me.
Attachment #170840 - Flags: review?(cbiesinger) → review+
Nelson, I think this patch doesn't break binary compatibility because the functions are still there. I'd like another pair of eyes to review it. Please confirm. Thanks.
Comment on attachment 170840 [details] [diff] [review] patch v1.1 second review = nelson I believe this patch is correct for all the CPU types on which NSS runs. It's correctness on any CPU depends on the native 64-bit integer format being the same as the format of struct PRInt64. I believe that is true for all CPU types on which NSS presently runs, and is likely to remain true. >+#define LL_MAXINT 9223372036854775807L >+#define LL_MAXUINT 18446744073709551615UL I would have preferred to see 0x7FFFFFFFFFFFFFFF instead of 92233...07L because I could tell at a glance that the hex form is correct, and need to use tools to verify that the decimal form is correct. But I have verified that now, so it's OK.
Attachment #170840 - Flags: superreview+
Nelson, you misunderstood what the change was because you thought PRInt64 is defined as a struct on all platforms. If a platform has a 64-bit integer type, PRInt64 is defined as that type. PRInt64 is defined as a struct only on the few platforms that don't have a 64-bit integer type. So the change is: 1. No change on platforms where PRInt64 is a struct. 2. On platforms where PRInt64 is a 64-bit integer type, - LL_MAXINT was defined as a function call - LL_MAXInt is now defined as a constant
No, I had no such misunderstanding. The issue is binary compatibility between code that uses native PRInt64 types and code that uses the structs. The issue is whether the values given by the new #defines be compatible with old code that uses the functions. Imagine new code using old shared libs, or old code using new shared libs, where the the new code treats PRInt64 as a native type and the old code treats it as a struct. For those combinations to continue to work and be compatible, it is necessary that the native types and the struct types place the data in memory in an identical fashion. I believe that they do that for all presently-supported CPUs.
Let me say that another way, not using the terms "old" and "new". Code that was compiled with HAVE_LONG_LONG should be interoperable with code that is not compiled with HAVE_LONG_LONG. I checked to see if it was, and am satisfied that it is.
Thanks for reviewing that. But this patch does not change the HAVE_LONG_LONG setting for any platform, hence my confusion about your review comments.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.