fix: Detect platform word size based on SIZE_MAX
#125
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The native word size on a platform matches the size of `size_t` with a few rare exceptions like the x32 ABI on linux. Therefore we can derive a general cross platform value for `BASE64_WORDSIZE` based on `SIZE_MAX` which the C99 standard guarantees to be available and well defined. This fixes the misdection of x86/arm32 win32 as a 64bit platform. See also https://en.cppreference.com/w/c/types/limits
BurningEnlightenment
force-pushed
the
dev/c99_word_size_detection
branch
from
09dc6b6 to
e826739
Compare
mayeut
reviewed
Jan 6, 2024
| #elif defined (__WORDSIZE) | ||
| # define BASE64_WORDSIZE __WORDSIZE | ||
| #elif defined (__SIZE_WIDTH__) | ||
| # define BASE64_WORDSIZE __SIZE_WIDTH__ | ||
| #elif SIZE_MAX == 0xffffffff |
There was a problem hiding this comment.
Suggested change
| #elif SIZE_MAX == 0xffffffff | |
| #elif SIZE_MAX == UINT32_MAX |
mayeut
reviewed
Jan 6, 2024
| #elif defined (__WORDSIZE) | ||
| # define BASE64_WORDSIZE __WORDSIZE | ||
| #elif defined (__SIZE_WIDTH__) | ||
| # define BASE64_WORDSIZE __SIZE_WIDTH__ | ||
| #elif SIZE_MAX == 0xffffffff | ||
| # define BASE64_WORDSIZE 32 | ||
| #elif SIZE_MAX == 0xffffffffffffffff |
There was a problem hiding this comment.
Suggested change
| #elif SIZE_MAX == 0xffffffffffffffff | |
| #elif SIZE_MAX == UINT64_MAX |
|
@BurningEnlightenment, I did not write anything related to |
This was referenced Jan 7, 2024
|
Merged with the fixes proposed by @mayeut. @BurningEnlightenment I slightly modified the commit title to add the issue number, and I modified the commit body to mark this issue and #123 as resolved. Hope you don't mind. |
|
@BurningEnlightenment I think it's a bit too late now to force-push to the master branch, so yes, please open a new PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The native word size on a platform matches the size of
size_twith a few rare exceptions like the x32 ABI on linux. Therefore we can derive a general cross platform value forBASE64_WORDSIZEbased onSIZE_MAXwhich the C99 standard guarantees to be available and well defined.This fixes the misdection of x86/arm32 win32 as a 64bit platform, i.e. it is an alternative to #123
@mayeut I think this should also cover the
__WORDSIZEand__SIZE_WIDTH__cases. Can you verify this? If that indeed is the case I would like remove those branches, too.See also https://en.cppreference.com/w/c/types/limits