Skip to content

fix memory leaks in map and bitmap#34

Merged
SergiusTheBest merged 1 commit intomainfrom
KF-20-fix-memory-leaks
Aug 4, 2025
Merged

fix memory leaks in map and bitmap#34
SergiusTheBest merged 1 commit intomainfrom
KF-20-fix-memory-leaks

Conversation

@Kellesi
Copy link
Collaborator

@Kellesi Kellesi commented Aug 1, 2025

Fix memory leaks in kf::map
Task: https://jira.dev.local/jira/browse/KF-20
Also faced to a little memory leak in kf::Bitmap and fixed it

Copy link
Member

@SergiusTheBest SergiusTheBest left a comment

Choose a reason for hiding this comment

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

Please, split it into 2 separate PRs and add tests for them.

@Kellesi
Copy link
Collaborator Author

Kellesi commented Aug 4, 2025

This issue was detected by driver verifier. Should I create my own test for memory leaks?

@SergiusTheBest
Copy link
Member

This issue was detected by driver verifier. Should I create my own test for memory leaks?

@Kellesi If existing tests trigger memory leak then there is no need in creating another test.

@SergiusTheBest
Copy link
Member

Please, split it into 2 separate PRs and add tests for them.

@Kellesi What about this?

@Kellesi
Copy link
Collaborator Author

Kellesi commented Aug 4, 2025

@SergiusTheBest
Memory leaks are already triggered by existing tests and detected by Driver Verifier, so no additional test is needed to trigger it.
Also, if I split the map and bitmap fix into 2 PRs, then the bitmap PR would contain just one line, since memory leak occurs in only one case. So if it is necessary due to any PR standards, could you please share them with me? Because here it seems a bit redundant for me.

@SergiusTheBest
Copy link
Member

@Kellesi Mainly it's because one fix is good and can be merger right away and be used by other guys. And another fix requires further discussion. But we can do it later. They are both good enough to be merged. Thank you for the fix! 💪

@SergiusTheBest SergiusTheBest self-requested a review August 4, 2025 13:01
@SergiusTheBest SergiusTheBest merged commit 263d1b2 into main Aug 4, 2025
2 checks passed
@SergiusTheBest SergiusTheBest deleted the KF-20-fix-memory-leaks branch August 4, 2025 13:02
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.

3 participants