This bug was found by, of all people, woutersmits, while FREDding his happynewyear campaign. See http://www.hard-light.net/forums/index.php?topic=90069.0 for discussion. The crash was triggered by the use of ( validate-all-arguments ) as the entire sexp tree for one of his mission events.
This was a two-part fix. The first part was to ensure that every *-argument sexp (of which validate-all-arguments is but one) is actually used within a when-argument or every-time-argument tree. More specifically, every sexp operation that expects an argument handler should first check that the arg_handler node actually exists.
The second part was to partially revert one of Taylor's old commits:
Revision: 3644
Author: taylor
Date: Tuesday, October 24, 2006 9:17:24 AM
Message:
various bits of cleanup (warning messages, things that Valgrind complained about, etc.)
fix an out-of-bounds issue on Sexp_nodes[] which was leading to stack corruption
----
Modified : /branches/fs2_open_3_6_9/fs2_open/code/parse/sexp.cpp
This was intended to prevent a -1 array reference into Sexp_nodes[]. What Taylor did was to cause repeated calls of the parent_operator function to loop endlessly at the topmost parent. But there are certain edge cases (such as the one woutersmits discovered) where you need to know whether a parent is the topmost one. What Taylor should have done, and what this change does, is to keep the -1 as a valid return value but check for it in the places where it may cause an issue.
If the subsystem was <none>, sexp_beam_floating_fire() wasn't consuming the node, resulting in it also being parsed as the X coordinate (making it always zero, because atoi("<none>") == 0).
`multi_sexp_set_turret_primary_ammo()` and `multi_sexp_set_turret_secondary_ammo()` were using an uninitialized pointer instead of an uninitialized array, like every other instance of multi_get_string(). This is just begging to write to invalid memory.
This adds getter and setter SEXPs for turret ammo.
"turret-get-primary-ammo"
"turret-get-secondary-ammo"
"turret-set-primary-ammo"
"turret-set-secondary-ammo"
I also forgot to make turrets that don't have the "use multiple guns"
flag set switch banks if their first bank is empty, so that has been
fixed as well.
I don't know if I was just tired when I wrote this originally or what, but there were two problems here; one, the for-loop condition was inverted (and since I made it a for-loop instead of a while-loop, I'm not sure why I didn't take out the "break" condition), so its contents would never actually evaluate. Two, the node matching random_argument wasn't being stored anywhere so even if the for-loop condition hadn't been wrong, it would've just used the last valid argument instead of the first argument every time. Fixes GitHub issue #84.
As requested by jg18, this improves the sound.tbl parser to properly handle +nocreate. If the file name of an entry should not be changed you have to specify <same>.
asarium