Skip to content
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

Change string map to hold key/value pairs #1

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Change string map to hold key/value pairs #1

merged 3 commits into from
Feb 28, 2023

Conversation

clalancette
Copy link

@clalancette clalancette commented Feb 27, 2023

This makes it easier for us to recover from errors.

I've also added in a test here to ensure that we are doing the right thing.


This change is Reviewable

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
This makes it easier for us to recover from errors.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
string_map->impl->keys = NULL;
allocator.deallocate(string_map->impl->values, allocator.state);
string_map->impl->values = NULL;
__clear_string_map(string_map);
Copy link

Choose a reason for hiding this comment

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

This call isn't strictly necessary, because if the size were non-zero, we wouldn't end up at this line. I think I'll remove it, but let me know if you prefer to keep it in just in case, then I'll re-add it.

@nnmm nnmm merged commit 43c8d30 into ApexAI:string-map-leak Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants