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

Fix: leaks and semantics, testing for memory leaks #64

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

kmapb
Copy link
Contributor

@kmapb kmapb commented Jan 7, 2024

Writing some leak-checking found a few bugs and correctness issues.

sz_string_init_from would sometimes leave a residue of noise in the space field of the new string. Lightly refactor to share some of the logic so it is easier to see that all four fields are set for both shapes of string.

The copied code in move constructor and assignment shared some issues: the target for the move didn't actually get initialized with the values of the source. For the case of move assignment where the target already held values, we need to release the associated memory.

I'm happy with the test code for this; while this is probably not a complete inventory of memory leaks, we have an easy way to add tests for new ones we encounter.

Writing some leak-checking found a few bugs and correctness issues.
@ashvardanian
Copy link
Owner

Thank you for the contribution, @kmapb! Will merge it in the morning, together with the remaining constructors.

@ashvardanian ashvardanian self-assigned this Jan 8, 2024
@ashvardanian ashvardanian merged commit 0174ed1 into ashvardanian:main-dev Jan 9, 2024
1 check passed
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