bugfix: NH::HashString Memory Violation #44
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s a temporary measure to keep its lifetime under control before larger refactoring is possible
…string` and make some code const-correct
piotrmacha
changed the title
…ad pool to single thread.
muczc1wek
approved these changes
Aug 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
zBassMusic was crashing in very specific conditions because
NH::HashStringcomparison withstd::hash<NH::HashString>resulted in illegal memory access.Step 1
Replace
const char*withstd::stringforNH::HashStringValue as a temporary measure to keep its lifetime under control before larger refactoring is possibleResult
Still broken because of other parts.
Step 2
Global refactoring to completely remove
NH::HashStringin favor ofstd::string. Because such refactor already spanned the whole project, we also replaced some uses ofUnion::Stringand many classes got rid of dependency on union-api, which is the work planned for 0.4.0. Alongside implementingstd::stringas the base string class, we also fixed some const-correctness issues andNH::namespace goes in the right direction.Result
Internal smoke tests passed green. We are waiting for confirmation from the person who reported the bugs. When we get the confirmation, this part of refactoring will be released as 0.3.4 bugfix.