-
Notifications
You must be signed in to change notification settings - Fork 349
zephyr: alloc: Use generic is_heap_pointer for shared and L3 heaps #10436
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
Replace the is_shared_buffer_heap_pointer function with the generic is_heap_pointer for shared heap. This change simplifies the memory handling path by relying on a common utility instead of a specialized function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
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 refactors the heap pointer validation logic by replacing specialized functions (is_shared_buffer_heap_pointer and is_l3_heap_pointer) with the generic is_heap_pointer utility. The change eliminates code duplication while maintaining identical functionality for memory deallocation checks in both shared and L3 heaps.
Key Changes:
- Removed two specialized heap pointer check functions in favor of the generic
is_heap_pointer - Updated
rfree()to useis_heap_pointerfor both L3 and shared buffer heap validation - Added initialization of
init_memandinit_bytesfields in heap structures to support the generic pointer check
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace the is_l3_heap_pointer function with the generic is_heap_pointer for L3 heap. This change simplifies the memory handling path by relying on a common utility instead of a specialized function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
c31502a to
78f0fd5
Compare
lyakh
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.
good to use a single function of course, but relying on .init_mem and .init_bytes is a bit concerning, a comment in zephyr/include/zephyr/sys/sys_heap.h says about them:
Would be good to clean this up and put the two values somewhere else
But I guess if they do get removed, we'll store the values elsewhere
/* Note: the init_mem/bytes fields are for the static initializer to
* have somewhere to put the arguments. The actual heap metadata at
* runtime lives in the heap memory itself and this struct simply
* functions as an opaque pointer. Would be good to clean this up and
* put the two values somewhere else, though it would make
* SYS_HEAP_DEFINE a little hairy to write.
*/This comment was added more than 6 years ago (zephyrproject-rtos/zephyr@aa42277) and it seems unlikely that these fields will be replaced in the future. They are still used by heaps created with the |
tmleman
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.
LGTM
|
Only known fails in CI, proceeding with merge. |
This pull request replaces specialized heap pointer checks with the generic
is_heap_pointerutility for both shared and L3 heaps. The change removes theis_shared_buffer_heap_pointerandis_l3_heap_pointerfunctions, simplifying the memory handling path and reducing code duplication. By relying on a common utility, the implementation becomes more consistent and easier to maintain without altering existing behavior.