-
Notifications
You must be signed in to change notification settings - Fork 586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use fixed-width int types for portability #66
Comments
Are we sure it is |
Its long and unsigned long that are all over libsnark, and which are 64
bits on linux but 32 bits on Windows (even Win64)
…On Jan 27, 2017 2:40 PM, "Daira Hopwood" ***@***.***> wrote:
Are we sure it is long long (rather than long) that is 32 bits? C99
requires it to be at least 64 bits.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#66 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AF9e0Lm3gyHcATjrC5VhYIjI0dmcCdFbks5rWmRmgaJpZM4LvIpE>
.
|
To add onto what @radix42 mentioned.. Even size_t is potentially problematic (32 bits on Win64 vs 64 bits on Linux/Unix). |
The difference in the size of Platform dependencies may arise when it needs to be cast into other integer types (or in overloading); but these should be caught at compile/link time, and in this case it makes sense to have an explicit cast. See discussion and example in zcash/zcash#1240. |
Libsnark currently uses
long
longandunsigned long
longin many places. On native Windows build, these are 32-bit types (even on 64-bit Windows), which breaks compatibility (#26) and in some cases correctness (e.g., #13).Convert all of these to C++11 fixed-width integer types:
int64_t
anduint64_t
.Except where they should be
size_t
instead (e.g., zcash/zcash#1240).(Ongoing work by @radix42 and @joshuayabut.)
The text was updated successfully, but these errors were encountered: