Fix a few endianness bugs on HTTP/2 branch #567
Merged
Conversation
This file contains hidden or 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
* get31bit now works on both little-endian and big-endian architectures * get31bit now explicitly expects its byte[] arg in network byte order * stream identifiers now encode correctly * poke16 function now writes bytes in network byte order (big-endian) * poke32 function now writes bytes in network byte order (big-endian)
|
Thanks! |
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.
Also stripped a few instances of trailing whitespace, in keeping with the project's whitespace policy in its .editorconfig file.
Caution: I have not yet been able to test these changes on a big-endian architecture; I thought that the Raspberry Pi, which uses an ARM chip, would be big-endian, but after downloading and setting up a Raspberry Pi emulator, I discovered that the Pi's chip is bi-endian (able to run in either mode), and is set to little-endian by default. So these changes are untested, but I believe them to be correct.
Certainly the
get31bitfunction should be using&&& 127, not&&& 128, if the goal is to strip the most significant bit and leave the other bits alone. (And it's better to guarantee which byte it's going to be stripping, hence my change to when thecheckEndiannesscall happens). The changes I'm least sure of are the changes topoke16andpoke32. I believe, from what I've read about big-endian architectures, that thebyte (n >>> 24)logic will produce the most significant byte on both little- and big-endian architectures, but that's the part I've been unable to test since I haven't yet found a big-endian system to emulate. However, I do believe that it's best if those two functions always poke their bytes into arrays in network byte order, and that's how I wrote them in this patch.