rollup: fixup CI and endianness conversion improvements #197
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
Add read/write into benchmark for u16 to test how the code behaves if the type is smaller than the native register size and for i64 to test that there’s nothing weird going on with signedness conversion.
Replace unsafe_read_slice macro which first copies data to destination buffer and then swaps endianess if necessary with a read_slice macro which does the swapping in a single pass while the data is read. This is done with `[T]:chunks_exact` which splits source into chunks which can be converted into integer of desired type with from_xx_bytes method. Closes #189, Closes #196
The compiler is smart enough that it notices when endianess conversion is not necessary and it simplifies copying with a conversion into plain memcpy. This means there’s no need to check target_endian and use unsafe_write_slice_native macro.
Firstly, get rid of $size argument. The size can be determined from $ty so there’s no need to pass it as a separate argument. Secondly, change it to use to_xx_bytes methods instead of Self::write_uxx so that it resembles read_slice.
unsafe_write_num_bytes macro can be replaced by a perfectly safe code which uses `n::to_xx_bytes()` to convert given number to desired representation and `buf[..N].copy_from_slice(...)` which writes that into the buffer. Closes #193
Rewrite read_uint and read_uint128 methods such that they no longer use unsafe code. Rather than casting pointers and doing unsafe copies, declare output byte buffer for the read number and use from_xx_bytes method to convert those read bytes to a number. Closes #192
This is seemingly needed for the soundness fix for #194. I spent zero time trying to find what the real MSRV was or to find another way to fix the soundness bug while maintaining the 1.41 MSRV because I didn't care to. Rust 1.60 is conservative enough as it is.
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.
Closes #189