Conversation
taz-mcrae
left a comment
taz-mcrae
left a comment
There was a problem hiding this comment.
Overall a really nice Rust wrapper of the C library. The code is simple and clear as the right wrapper should be. It contains also detailed documentation , only rustdoc for public methods is missing.
|
|
||
| ``` | ||
| cargo build | ||
| cargo run |
There was a problem hiding this comment.
Just a minor thing, you have 2 separate rust projects (two Cargo.toml files) rather than a single project with examples. If you have a single rust project, there is a standard approach to run examples #cargo run --example <example_name>. But a separate example project is easier to take and integrate by the user.
| counterintuitive, it reflects the fact that the object’s internal state changes | ||
| frequently, particularly when managing context, such as acquiring or releasing | ||
| locks. This behavior is mandated by the underlying C library interface, and | ||
| therefore cannot be avoided. |
There was a problem hiding this comment.
Just a remark: in your use-case the mutable reference to self makes more sense, but there is also a way how to avoid a mutable reference in every method: RWLock.
| └── src | ||
| ├── lib.rs | ||
| ├── bindings.h | ||
| └── bindings.rc |
| direct use in Rust applications. Instead, it serves as the foundation for the | ||
| higher-level wrapper located in the lib.rs file. | ||
|
|
||
| **Note:** The Rust compiler emits warnings regarding the use of 128-bit integers |
There was a problem hiding this comment.
That's great that you explain why you suppress some warnings 👍
| Ok(UtaApiV1{api, context: vec![0u8; context_size]}) | ||
| } | ||
|
|
||
| pub fn derive_key(&mut self, len_key: usize, dv: &[u8], key_slot: u8) -> Result<Vec<u8>, UtaError> { |
There was a problem hiding this comment.
Even though these library functions are pretty clear, it would be nice to add to each of them at least a basic rustdoc description (you can use LLM for that) to get a nice cargo doc documentation of the library.
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
It is great to have a unit test for each public function 👍
|
Due to organizational reasons and changes in the file structure the new pull request #33 was created. |
This contribution provides a refined version of the Rust wrapper from PR #30. This version implements several improvement suggestions from the previous PR and some additional finding by GitHub Copilot. The robustness of the code is improved by fine tuning and polishing. Also the file structure was changed and the two crates were merged into one. Note: what is still missing is automatic testing of the code in the pipeline.
2 participants
This contribution provides a Rust wrapper
implemented in the crate
libuta_rust, along with example usage in theexamplescrate.Note: what is still missing is automatic testing of the code in the pipeline.