ULL constants are not C89 #745
Comments
real-or-random
added a commit
to real-or-random/secp256k1
that referenced
this issue
Apr 22, 2020
This commit is the result of creating a file constants.sed with contents ``` s/(0x([A-F]|[0-9])+)ULL/UINT64_C(\1)/gi s/(0x([A-F]|[0-9])+)UL/UINT32_C(\1)/gi s/(([0-9])+)ULL/UINT64_C(\1)/gi s/(([0-9])+)UL/UINT32_C(\1)/gi s/(([0-9])+)LL/INT64_C(\1)/gi ``` and running `sed -E -i -f constants.sed src/*.[ch]` (and fixing custom indentation in four affected lines). Use `git grep -iE '([A-F]|[0-9])UL'` to verify the result. Moreover, this commit removes `-Wno-long-long` from CFLAGS, which is no longer needed then. It also removes `-Wno-overlength-strings`, which is apparently not needed currently either. The motivation for this change is compliance with C89 (bitcoin-core#745) for ULL/LL but also reviewability: Even though it's not relevant in the current code, I think it's confusing to use the portable uint32_t/uint64_t types but then constants of the unsigned long (long) types, which differ across targets. Fixes bitcoin-core#745.
|
long long is specified in C99, so possible fix would be just to replace C89 with C99 in the readme |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The README states as a goal: "Intended to be portable to any system with a C89 compiler and uint64_t support."
The C89 syntax does not specify
unsigned long long(ULL) constants because there is no requirement that aunsigned long longtype exists. We useULLin a few places, e.g.:secp256k1/src/scalar_4x64_impl.h
Line 11 in 39198a0
We should replace
ULLwith theUINT64_Cmacro from instdint.h. Note thatULis not an issue: It's in the C89 syntax andunsigned longis guaranteed to have at least 32 value bits, so we can use it foruint32_tconstants. We may still want to change it for consistency and readability (clearer expression of intent).Alternatively, we can do nothing (because noone seemed to care about this so far).
The text was updated successfully, but these errors were encountered: