This repository has been archived by the owner on May 18, 2022. It is now read-only.
Remove byteorder dependency #124
Merged
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
daboross
force-pushed
the
remove-byteorder
branch
2 times, most recently
from
7aab587 to
6e3f0ef
Compare
jonas-schievink
approved these changes
Jul 10, 2020
This implements FromBytes and ToBytes for the remaining Header type without these impls, and changes one place where we were writing the Header to a ByteWriter to uses to_bytes rather than to_u16 + LittleEndian::write_u16. Closes jonas-schievink#24. Signed-off-by: David Ross <[email protected]>
daboross
force-pushed
the
remove-byteorder
branch
from
6e3f0ef to
ed14ccd
Compare
|
Sounds good! Rebased. |
There are a few places this ends up more verbose, but it removes a dependency & source of unsafe code. The new-ish std methods cover almost everything we were using byteorder for, in any case. Signed-off-by: David Ross <[email protected]>
daboross
force-pushed
the
remove-byteorder
branch
from
ed14ccd to
7066bf5
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
I included a fix for the last remaining header not implementing
FromBytes/ToBytesin this PR, as that made removingbyteordereasily. This could also be submitted separately, if you want.I think the majority of places
byteorderwas used were more than well covered by the newfrom/to_le/be_bytesmethods on integers. A few places where we're actually using it to read/write to byte slices needed more changes, mainly usingcopy_from_sliceortry_intoto convert&[u8]to[u8; N].Let me know how this looks?
Fixes #24. CC #122.