Capsules: SipHash respect leasable buffers and handle arbitrary buffer sizes #3506
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
8000
468f316 to
21ba455
Compare
|
These changes look good to me except for my complaint about the leasable buffer change on the other PR with those changes (#3505 (comment)) |
c7aa1d9 to
a1d11f4
Compare
|
I rebased this and removed the leasable buffer api commit. |
brghena
approved these changes
Jul 18, 2023
lschuermann
approved these changes
Jul 18, 2023
Comment on lines
-152
to
+156
| let (int_bytes, _rest) = input.split_at(mem::size_of::<u64>()); | ||
| u64::from_le_bytes(int_bytes.try_into().unwrap()) | ||
| let mut eight_buf: [u8; 8] = [0; 8]; | ||
| for i in 0..8 { | ||
| eight_buf[i] = *input.get(i).unwrap_or(&0); | ||
| } | ||
| u64::from_le_bytes(eight_buf) |
There was a problem hiding this comment.
I think this looks hideous, but it really seems like Rust doesn't have a better story to collect an iterator into a fixed size array yet...
a1d11f4 to
cd5633c
Compare
Do not assume data is at the start of the buffer.
cd5633c to
1481f9b
Compare
|
Rebased. |
|
Ok, sorry, I thought this would pass CI on its own. But it doesn't any trying to figure out which changes from the full PR (#3508) make it compile seems like too much work. We'll have to just go with the full PR at this point. These commits are verbatim in that PR, so it could still be helpful to look at this change set. |
|
Merged in #3508. |
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.
Pull Request Overview
This pull request updates the siphash capsule to correctly use leasable buffers and to accept data in units not a multiple of 8 bytes.
Testing Strategy
Updating the k-v stack and comparing hashes to an online calculator.
TODO or Help Wanted
n/a
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.