Crash when dismissing Dead Soldier

[Rinne]

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

[Rinne]

My C++ isn't very good, but I would expect a null pointer error in the StrategicRemoveMerc function (Merc_Contract.cc, line 615)

[Rinne]

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();
}

[Rinne]

Changed priority to minor since a workaround exists by using the assignment column

[Rinne]

Fixed by Patch https://bitbucket.org/gennady/ja2-stracciatella/pull-request/3/fixed-issue-114/diff

[Rinne]

Reopened till patch acceptance