The purpose of this function is to provide modders the capability to jump to a mission using LUA scripts. There is an optional argument for jumping to a hub mission, a good example is the JAD campaign map.
We were declaring an array of NUM_TURRET_ORDER_TYPES ints, and then only specifying the first oindex of them; the memcpy() could've been changed to multiply by oindex instead, but it seemed cleaner to just reset new_target_order to -1 in all values and get rid of the redundant per-ship reset.
Discovered by triggering an Int3() in aiturret.cpp (line 1167 as of this writing) and finding uninitialized values in the turret_targeting_order array.
Affected SEXPs include turret-set-target-order, turret-set-direction-preference, turret-set-rate-of-fire, turret-set-optimum-range, ship-turret-target-order, turret-get-primary-ammo, turret-set-primary-ammo, turret-get-secondary-ammo, and turret-set-secondary-ammo.
It's worth noting that while turret-set-direction-preference, turret-set-rate-of-fire, and turret-set-optimum-range were wrong, they were wrong in a way that wouldn't actually affect the outcome most of the time (thanks to the "if(turret == NULL)" checks simply skipping invalid subsystem names).
Should result in reduced BMPMAN slot usage with certain scripts as bm_unload() unloads the image data but holds on to the slot, while bm_release() releases the slot.
In 9ec10dd I thought I was fixing a memory leak, but the actual scenario was that the freeing was just delayed until later. This removes the freeing added by 9ec10dd and changes the free() calls into vm_free() calls.
Previously, the "this != NULL" checks in the isValid() methods were covering most of these cases, but they were removed as part of PR #274 because clang was generating warnings because calling a method on a null pointer is undefined behavior (so the condition *should* be tautological). This performs the checks outside of isValid(), which should avoid (at least this variety of) undefined behavior while fixing the crashes that dereferencing null pointers was causing.
ssm_create() subtracts the start coordinates from the position of the target, and then normalizes that. Since call-ssm-strike was using the position of the target as the start coordinates, this was obviously resulting in normalizing a null vec3d, generating the spurious warning. This just makes call-ssm-strike use a point immediately behind the target's position (using the target's orientation) as the starting coordinate, thereby avoiding the warning.
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.
DahBlount