Open
Conversation
m4tx
requested changes
Mar 15, 2026
Member
m4tx
left a comment
m4tx
left a comment
There was a problem hiding this comment.
Please have a look at the comments, and fix the clippy problems.
| #[derive(Eq, PartialEq, PartialOrd, Ord, Hash)] | ||
| pub struct SecureArray<T, const LENGTH: usize> | ||
| #[derive(Eq, PartialOrd, Ord, Hash)] | ||
| pub struct SecureArray<const LENGTH: usize> |
Member
There was a problem hiding this comment.
What's the reason for removing the T generic here? If we want to keep it this way, the name is misleading. This is not SecureArray anymore, but something more like SecureFixedByteBuffer now.
| /// Be careful with `SecureVec::from`: if you have a borrowed string, it will be copied. | ||
| /// Use `SecureVec::new` if you have a `Vec<u8>`. | ||
| #[derive(Eq, PartialOrd, Ord, Hash)] | ||
| pub struct SecureVec { |
Member
There was a problem hiding this comment.
Same question here, this is not SecureVec now. Please either change the name, or restore the generic parameter (feels like there's no real reason for removing the generic paramter, though)
| unsafe { | ||
| std::str::from_utf8_unchecked_mut(self.0.unsecure_mut()) | ||
| } | ||
| // TODO: fix |
| impl<const LENGTH: usize> From<[u8; LENGTH]> for SecureArray<LENGTH> | ||
| where | ||
| T: Copy + Zeroize, | ||
| u8: Copy + Zeroize, |
Member
There was a problem hiding this comment.
Where's this generic bound coming from? This is always true for u8 (to be honest, I would except to compiler to throw an error at this)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Local fork of
secure-stringto get it working for cot-rs/cot#328