-
Notifications
You must be signed in to change notification settings - Fork 101
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
Fix a leak in test_array_list.cpp #149
Fix a leak in test_array_list.cpp #149
Conversation
Before this change, test_array_list ASAN was reporting 280 bytes leaked in 4 allocations. After this patch is applied, test_array_list does not emit any ASAN related warning anymore. Fixes ros2#148 Signed-off-by: Thomas Moulard <tmoulard@amazon.com>
Before:
After:
|
@wjwwood another one fixing leaks in tests |
Signed-off-by: Thomas Moulard <tmoulard@amazon.com>
I tried to apply your comments from the other PR here. Note that the situation is bit different as there already was a fixture to initialize the data structure. I kept the pattern in the TearDown allowing |
Signed-off-by: Thomas Moulard <tmoulard@amazon.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.
lgtm, other than the pre-existing issue I pointed out.
I think I fixed everything and it should be OK to merge if CI is green. |
@wjwwood Should be ok to merge! |
Before this change, test_array_list ASAN was reporting
280 bytes leaked in 4 allocations. After this patch
is applied, test_array_list does not emit any ASAN related
warning anymore.
Fixes #148
Signed-off-by: Thomas Moulard tmoulard@amazon.com