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 for hedit 'x' crash, v.2 #745

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

jdevnull
Copy link
Contributor

  1. When host changes are saved, we call collate_host_entrances to rebuild entrance lists. Thus, when deleting host A, we don't need to search other hosts for entrances pointing to host A. This solves the issue of trying to find an entrance in another host that may not (yet) exist.

  2. It makes sense to clear host A's own list of entrances as part of free_host instead of in hedit.

  3. When making a copy of an existing host for editing in olc, we don't want the copy to point to the existing host's list of entrances. This solves the issue where we can end up with a pointer directing us to freed memory. We also don't need to make a copy of the list because collate_host_entrances will rebuild the list if/when we save, so we can just set it to null.

src/hedit.cpp Outdated Show resolved Hide resolved
@luciensadi
Copy link
Owner

Looks like the crashes were caused by the entrance linked list not being cloned on hedit. This caused the actual host's entrances to be blown away when the edited host was extracted, which broke memory constraints.

@jdevnull
Copy link
Contributor Author

jdevnull commented Jun 18, 2024

Yeah, this was what I was trying to describe in part 3 of the PR description. That said, we don't actually need to clone the entrance list, since they're going to be rebuilt anyway - we just need the copy to not point to the actual host's linked list, so we can just assign it to null.

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.

None yet

2 participants