-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Equeue chaining bug fixes #9066
Conversation
report error / assert when allocation fails.
Issue was seen with below example EventQueue q1; EventQueue q2; void main() { while( true ) { q1.chain( &q2 ); // Chain q2 to q1 q1.chain( NULL ); // Remove chain from q1 //This second step should free the memory from the chained q2 event. } } Memory allocated from q1 slab was freed for q2, which will result in memory leak.
@ARMmbed/mbed-os-maintainers - API was updated to return error codes. Should be considered for minor release. |
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.
I'd say this is API change is fine to include in a fix. Doesn't affect ABI, so won't upset any binary blobs using it.
Only things I can imagine it tripping are people inheriting from EventQueue
and overriding, or some sort of unit test stub.
fcf4758
to
87c557c
Compare
Patch or minor, will leave the decision to @ARMmbed/mbed-os-maintainers |
Updated for a patch. The changing return types are necessary for the fix, and the risk for these updated return types is low. |
@geky @SenRamakri - Please review |
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 good! Thanks for this
CI started |
Test run: FAILEDSummary: 1 of 1 test jobs failed Failed test jobs:
|
Fixed in 14606b5 |
CI started |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Description
Fixes issues reported in #8567
Pull request type
CC @geky @marcemmers