-
Notifications
You must be signed in to change notification settings - Fork 25
Flash Ram Sim no malloc support. Integer sign fixes #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bigbrett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
dff533f to
eca1a12
Compare
billphipps
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent catch on the signed values. I found some other messages that had errors in them too, but I'll fix those seperately.
Consider eliminating malloc() within this library by updating the test/examples/tools to malloc instead.
Just need a few parameter validations and I think we are probably good!
billphipps
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of lingering free()'s. Looks great!
Please pull before you do a final push to resolve the conflict in test_cert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes malloc dependencies from the Flash RAM Simulator library and fixes integer sign mismatches for error codes. The changes enable usage in environments without dynamic memory allocation and address TI ARM compiler warnings about assigning negative error codes to unsigned integers.
- Adds a
memoryfield towhFlashRamsimCfgto allow user-provided memory allocation instead of malloc - Changes
uint32_treturn code fields toint32_tin SHE message structures to properly handle negative error values - Updates the
nfObject_Offsetfunction to return error codes properly instead of mixing offsets with error values
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfhsm/wh_flash_ramsim.h | Adds memory field to configuration structure |
| wolfhsm/wh_message_she.h | Changes return code fields from uint32_t to int32_t |
| src/wh_flash_ramsim.c | Removes malloc dependency and uses user-provided memory |
| src/wh_nvm_flash.c | Fixes nfObject_Offset to return errors via output parameter |
| test/*.c | Updates test files to provide static memory buffers |
| examples/*.c | Updates examples to provide static memory buffers |
| benchmark/*.c | Updates benchmark to provide static memory buffer |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
07aa125 to
59b958b
Compare
… to top of function.
59b958b to
e6c1486
Compare
billphipps
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks!
Flash RAM Sim changes
Currently
whFlashRamsim_Initusesmallocto allocate the simulated NVM space in RAM. This may be cumbersome for environments where dynamic memory allocation is not implemented or unavailable. This PR implements the following to address this:memoryfield towhFlashRamsimCfgstruct to allow the user to specify a memoryInteger sign fixes
There were multiple instances
uint32_tvariable, return value, and/or field that gets set to an error code which are negative values. The TI arm compiler throws an error for this that can't be turned off or ignored. In this PR I changed these occurrences to use a signed integer instead.