Skip to content

[CRASHFIX] Stop vehicle missing frame check from causing crash by using typical part iteration loop. Optimize too. #3500

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

Merged
merged 2 commits into from
Oct 8, 2013

Conversation

atomicdryad
Copy link
Contributor

  • Fix crash in vehicle::add_missing_frames

* Prevent vehicle::add_missing_frames from needlessly triggering precalc_mounts/find_* etc multiple times

gcc linux with -O3 results in:

Program received signal SIGSEGV, Segmentation fault.
vehicle::add_missing_frames (this=0x8972df0) at vehicle.cpp:99
99 int next_y = it->mount_dy;
(gdb) bt
#0 vehicle::add_missing_frames (this=0x8972df0) at vehicle.cpp:99
#1 0x0853d0f2 in vehicle::load (this=0x8972df0, stin=...) at vehicle.cpp:84
#2 0x082913a7 in mapbuffer::unserialize (this=0x865a040, fin=...) at mapbuffer.cpp:412
#3 0x08291a2a in mapbuffer::load (this=0x865a040) at mapbuffer.cpp:248
#4 0x0827c27c in main (argc=0, argv=0xbffff7a4) at main.cpp:87

(gdb)

This fixes the crash in -O3 and unoptimized.

* run after part insertion, within json_load/load_legacy functions
* push new frame into item list instead of using vehicle::install_part, which triggers
  find_external_parts, find_exhaust, precalc_mounts once for every frame added; load
  does this anyway.
* Only run if savegame_loading_version is v10 or below; new saves won't have vehicles
  with invalid frame config.
@ianestrachan
Copy link
Contributor

Odd - I tested this by pulling an old version, saving the world, then loading it in the new version, and it added the extra frames...

...but now I see it looks like I was modifying a list while iterating over it. Ouch. Yeah, that would definitely cause problems. Thanks for fixing this.

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