- edited description
Crash when dismissing Dead Soldier
First of all I would like to say thank you for reviving this splendid project :)
When I try to remove / dismiss a dead soldier, or a destroyed robot, the game crashes with a segfault.
I have attached a savegame, although it can be tested by starting a new game, hiring two Mercs and killing one as well.
System: Gentoo Linux x64 Compiled with GCC 4.7.3
Comments (9)
-
reporter -
reporter - edited description
-
reporter My C++ isn't very good, but I would expect a null pointer error in the StrategicRemoveMerc function (Merc_Contract.cc, line 615)
-
reporter I have isolated the Problem:
The error does indeed happen in StrategicRemoveMerc at line 623, when the function first wants to access s (Soldiertype).
There are three ways to Remove a Merc from the List:
1.: Opening the Assignment Box for a Dead Merc (in the ASSIGN-Column) and selecting "Remove MERC"
2.: Opening the Departure Box for a Dead Merc (in the DEP-Column) and selecting "Remove MERC"
3.: Opening the Departure Box for a living Merc (in the DEP-Column) and selecting "Dismiss"
Option 1 and 2 use the same function (StrategicRemoveMerc in Merc_Contract.cc). Option 3 uses a different function, that I don't know.
The game does only crash with option 2, but works fine with option 1.
Reason:
The function StrategicRemoveMerc is called by RemoveMercMenuBtnCallback in Assignments.cc at line 4398 and passed the currently assigned Soldier pSoldier. pSoldier is assigned at line 4367 by calling GetSelectedAssignSoldier. GetSelectedAssignSoldier gathers the current Soldier by looking up bSelectedAssignChar. The Problem is, that bSelectedAssignChar is not set if the DEP-Column is selected, but only if the ASSIGN Column is selected, thus returning -1 and leading to a null pointer returned in GetSelectedAssignSoldier.
The relevant Code Blocks:
static void RemoveMercMenuBtnCallback(MOUSE_REGION* pRegion, INT32 iReason) { // btn callback handler for contract region INT32 iValue = -1; SOLDIERTYPE * pSoldier = NULL; pSoldier = GetSelectedAssignSoldier( FALSE ); // pSoldier Assignment iValue = MSYS_GetRegionUserData( pRegion, 0 ); if (iReason & MSYS_CALLBACK_REASON_LBUTTON_UP) { switch( iValue ) { case REMOVE_MERC_CANCEL: ... // blabla case( REMOVE_MERC ): bSelectedInfoChar = -1; StrategicRemoveMerc(*pSoldier); // StrategicRemoveMerc call // dirty region fCharacterInfoPanelDirty = TRUE; fTeamPanelDirty = TRUE; ... // blabla } } }
static SOLDIERTYPE* GetSelectedAssignSoldier(BOOLEAN fNullOK) { SOLDIERTYPE *pSoldier = NULL; if (fInMapMode) { // mapscreen version if (bSelectedAssignChar >= 0 && bSelectedAssignChar < MAX_CHARACTER_COUNT) // This is -1 if the DEP-Column is selected { pSoldier = gCharactersList[bSelectedAssignChar].merc; } } else { ... // bla return( pSoldier ); }
void StrategicRemoveMerc(SOLDIERTYPE& s) { if (gfInContractMenuFromRenewSequence) { EndCurrentContractRenewal(); } // ATE: Determine which HISTORY ENTRY to use... if (s.ubLeaveHistoryCode == 0) // This is where the null pointer segfaults { // Default use contract expired reason... s.ubLeaveHistoryCode = HISTORY_MERC_CONTRACT_EXPIRED; } UINT8 const ubHistoryCode = s.ubLeaveHistoryCode; if (s.bLife <= 0) { // The soldier is dead AddCharacterToDeadList(&s); } else if... // We don't get to this point UpdateTeamPanelAssignments(); }
-
reporter - marked as minor
Changed priority to minor since a workaround exists by using the assignment column
-
reporter - removed responsible
-
reporter - changed status to resolved
-
reporter - changed status to open
Reopened till patch acceptance
-
repo owner - changed status to resolved
- Log in to comment