Skip to content

Conversation

@AlexLanzano
Copy link
Contributor

No description provided.

@AlexLanzano AlexLanzano self-assigned this Sep 2, 2025
@AlexLanzano AlexLanzano force-pushed the keywrap-feature branch 3 times, most recently from fd4ab14 to 7e85b55 Compare September 5, 2025 20:41
@AlexLanzano AlexLanzano marked this pull request as ready for review September 5, 2025 20:43
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great start! I think there are few items we should discuss from a design perspective. Let me know about my comments and how you want to proceed

Copy link
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete review - just some quick nits up front about naming and organization

@AlexLanzano AlexLanzano force-pushed the keywrap-feature branch 2 times, most recently from 9164df3 to 845a276 Compare September 13, 2025 17:38
Copy link
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work @AlexLanzano. A few final tweaks and it is good to go

- Add braces to all braceless if statements
- Validate group, action, and response size on all key wrap response
  handling functions
- Replace non-portable bool type with int
- Misc style fixes
Some of the static strings in wolfHSM aren't accounting for the
terminating NULL character which throws a
"unterminated-string-initialization" error in GCC version 15.2.1.
Address this by either changing the string to an array or increasing the
string array size
@bigbrett bigbrett dismissed billphipps’s stale review September 23, 2025 19:49

changes addressed

@bigbrett bigbrett merged commit 1124aa8 into wolfSSL:main Sep 24, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants