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

Warn about loading dead bodies, but handle safely. #5167

Merged

Conversation

fluffyfreak
Copy link
Contributor

Save games could fail to load due to the body index being written as (Uint32)-1.
Handle getting an out-of-range / SDL_MAX_UINT32 value when trying to find a body by returning nullptr instead.

This allows savegames to load despite this issue.

The issue itself is caused by the AICmdFlyAround having a pointer to a Body *m_obstructor that it does not own and is not refcounted. This can be destroyed before/whilst saving, which fails to find the body ('cos it's gone), the GetIndexForBody returns (Uint32)-1, and upon loading it crashes.

I would like to have fixed the actual cause by this COVID19 vaccination has hit me like a bus and I feel like I've got flu.
So posting this as-is for possible merging, comments, and suggestions. Either other can fix it, or it can wait until my brain works again.

Copy link
Member

@Web-eWorks Web-eWorks left a comment

Choose a reason for hiding this comment

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

I would prefer returning INT32_MAX (and in general avoiding all SDL int-types in the codebase) but this is good to merge now and we can convert it to standard types at a later date.

@Web-eWorks Web-eWorks merged commit 18b4d48 into pioneerspacesim:master May 2, 2021
@fluffyfreak fluffyfreak deleted the SafeLoadBadBodyIndex branch May 20, 2021 19:44
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