Skip to content

MOZ-0: Fix memleak #40

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.

Sign up for GitHub

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

Jump to bottom
Open
sune-ext-lyngby wants to merge 1 commit into
base: mbed-os-6.7.0-patched
Choose a base branch
from
Open

MOZ-0: Fix memleak #40

sune-ext-lyngby wants to merge 1 commit into from

Conversation

@sune-ext-lyngby
Copy link

@sune-ext-lyngby sune-ext-lyngby commented May 24, 2022 

It looks like we have a memory leak if the event queue is full.
In that case, I haven't been able to find who should deallocate the
queue event object, so we that should probably happen here.

@sune-ext-lyngby
MOZ-0: Fix memleak
2d0d5c8 
It looks like we have a memory leak if the event queue is full.
In that case, I haven't been able to find who should deallocate the
queue event object, so we that should probably happen here.
@sune-ext-lyngby
Copy link
Author

sune-ext-lyngby commented May 24, 2022

Untested; haven't even built tested it

MichaelRasmussn
MichaelRasmussn approved these changes May 25, 2022
View reviewed changes
Copy link

@MichaelRasmussn MichaelRasmussn left a comment

Choose a reason for hiding this comment

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

LGTM

@sune-ext-lyngby
Copy link
Author

sune-ext-lyngby commented May 25, 2022

After reading and re-reading, it looks like that the id will always be valid, and the if block introduced in 24e93e6 is dead code. Though a bit more eyes would be nice.

I think we should roll those if blocks of that changeset back instead.

@sune-ext-lyngby sune-ext-lyngby mentioned this pull request May 25, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@beo-teis beo-teis Awaiting requested review from beo-teis

@hansbinderup hansbinderup Awaiting requested review from hansbinderup

@anedergaard anedergaard Awaiting requested review from anedergaard

@isircu isircu Awaiting requested review from isircu

@Emil-Juhl Emil-Juhl Awaiting requested review from Emil-Juhl

1 more reviewer

@MichaelRasmussn MichaelRasmussn MichaelRasmussn approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sune-ext-lyngby @MichaelRasmussn