Skip to content

Commit ce9d3d7

Browse files
Caball009Caball009
authored
[GEN][ZH] Fix undefined behavior in WindowLayout::destroyWindows (#878)
1 parent f5b8563 commit ce9d3d7

24 files changed

+170
-98
lines changed

‎Generals/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -954,8 +954,8 @@ ControlBar::~ControlBar( void )
954954
{
955955
m_scienceLayout->destroyWindows();
956956
deleteInstance(m_scienceLayout);
957+
m_scienceLayout = NULL;
957958
}
958-
m_scienceLayout = NULL;
959959
m_genArrow = NULL;
960960
if(m_videoManager)
961961
delete m_videoManager;

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Diplomacy.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,8 @@ void ResetDiplomacy( void )
292292
theLayout->destroyWindows();
293293
deleteInstance(theLayout);
294294
InitBuddyControls(-1);
295+
theLayout = NULL;
295296
}
296-
theLayout = NULL;
297297
theWindow = NULL;
298298
if (theAnimateWindowManager)
299299
delete theAnimateWindowManager;

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/DifficultySelect.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,12 @@ WindowMsgHandledType DifficultySelectSystem( GameWindow *window, UnsignedInt msg
258258
pref.write();
259259
//TheScriptEngine->setGlobalDifficulty(s_AIDiff); // CANNOT DO THIS! REPLAYS WILL BREAK!
260260
WindowLayout *layout = window->winGetLayout();
261-
layout->destroyWindows();
262-
deleteInstance(layout);
261+
if (layout)
262+
{
263+
layout->destroyWindows();
264+
deleteInstance(layout);
265+
}
266+
263267
setupGameStart(TheCampaignManager->getCurrentMap(), s_AIDiff);
264268
// start the game
265269
}
@@ -268,8 +272,11 @@ WindowMsgHandledType DifficultySelectSystem( GameWindow *window, UnsignedInt msg
268272
TheCampaignManager->setCampaign( AsciiString::TheEmptyString );
269273
TheWindowManager->winUnsetModal(window);
270274
WindowLayout *layout = window->winGetLayout();
271-
layout->destroyWindows();
272-
deleteInstance(layout);
275+
if (layout)
276+
{
277+
layout->destroyWindows();
278+
deleteInstance(layout);
279+
}
273280

274281
}
275282
else if ( controlID == radioButtonEasyAIID )

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/DownloadMenu.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,14 @@ static void closeDownloadWindow( void )
8282
if (!parent)
8383
return;
8484

85-
WindowLayout *menuLayout = parent->winGetLayout();
86-
menuLayout->runShutdown();
87-
menuLayout->destroyWindows();
88-
deleteInstance(menuLayout);
89-
menuLayout = NULL;
85+
WindowLayout *menuLayout = parent->winGetLayout();
86+
if (menuLayout)
87+
{
88+
menuLayout->runShutdown();
89+
menuLayout->destroyWindows();
90+
deleteInstance(menuLayout);
91+
menuLayout = NULL;
92+
}
9093

9194
GameWindow *mainWin = TheWindowManager->winGetWindowFromId( NULL, NAMEKEY("MainMenu.wnd:MainMenuParent") );
9295
if (mainWin)

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/LanMapSelectMenu.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,13 @@ WindowMsgHandledType LanMapSelectMenuSystem( GameWindow *window, UnsignedInt msg
344344
else if ( controlID == buttonBack )
345345
{
346346

347-
mapSelectLayout->destroyWindows();
348-
deleteInstance(mapSelectLayout);
349-
mapSelectLayout = NULL;
347+
if (mapSelectLayout)
348+
{
349+
mapSelectLayout->destroyWindows();
350+
deleteInstance(mapSelectLayout);
351+
mapSelectLayout = NULL;
352+
}
353+
350354
// set the controls to NULL since they've been destroyed.
351355
NullifyControls();
352356
showLANGameOptionsUnderlyingGUIElements(TRUE);
@@ -390,9 +394,12 @@ WindowMsgHandledType LanMapSelectMenuSystem( GameWindow *window, UnsignedInt msg
390394
}
391395

392396

393-
mapSelectLayout->destroyWindows();
394-
deleteInstance(mapSelectLayout);
395-
mapSelectLayout = NULL;
397+
if (mapSelectLayout)
398+
{
399+
mapSelectLayout->destroyWindows();
400+
deleteInstance(mapSelectLayout);
401+
mapSelectLayout = NULL;
402+
}
396403

397404
// set the controls to NULL since they've been destroyed.
398405
NullifyControls();

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupCommunicator.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,13 @@ WindowMsgHandledType PopupCommunicatorSystem( GameWindow *window, UnsignedInt ms
184184

185185
if( controlID == buttonOkID )
186186
{
187-
WindowLayout *popupCommunicatorLayout = window->winGetLayout();
188-
popupCommunicatorLayout->destroyWindows();
189-
deleteInstance(popupCommunicatorLayout);
190-
popupCommunicatorLayout = NULL;
187+
WindowLayout *popupCommunicatorLayout = window->winGetLayout();
188+
if (popupCommunicatorLayout)
189+
{
190+
popupCommunicatorLayout->destroyWindows();
191+
deleteInstance(popupCommunicatorLayout);
192+
popupCommunicatorLayout = NULL;
193+
}
191194
} // end if
192195

193196
break;

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/QuitMenu.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,14 @@ void destroyQuitMenu()
122122
{
123123
fullQuitMenuLayout->destroyWindows();
124124
deleteInstance(fullQuitMenuLayout);
125+
fullQuitMenuLayout = NULL;
125126
}
126-
fullQuitMenuLayout = NULL;
127127
if(noSaveLoadQuitMenuLayout)
128128
{
129129
noSaveLoadQuitMenuLayout->destroyWindows();
130130
deleteInstance(noSaveLoadQuitMenuLayout);
131+
noSaveLoadQuitMenuLayout = NULL;
131132
}
132-
noSaveLoadQuitMenuLayout = NULL;
133133
quitMenuLayout = NULL;
134134
isVisible = FALSE;
135135
}

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ScoreScreen.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -759,9 +759,12 @@ void finishSinglePlayerInit( void )
759759
TheInGameUI->freeMessageResources();
760760

761761
//
762-
s_blankLayout->destroyWindows();
763-
deleteInstance(s_blankLayout);
764-
s_blankLayout = NULL;
762+
if (s_blankLayout)
763+
{
764+
s_blankLayout->destroyWindows();
765+
deleteInstance(s_blankLayout);
766+
s_blankLayout = NULL;
767+
}
765768

766769
// set keyboard focus to main parent
767770
TheWindowManager->winSetFocus( parent );

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/SkirmishMapSelectMenu.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,13 @@ WindowMsgHandledType SkirmishMapSelectMenuSystem( GameWindow *window, UnsignedIn
521521
{
522522
showSkirmishGameOptionsUnderlyingGUIElements(TRUE);
523523

524-
skirmishMapSelectLayout->destroyWindows();
525-
deleteInstance(skirmishMapSelectLayout);
526-
skirmishMapSelectLayout = NULL;
524+
if (skirmishMapSelectLayout)
525+
{
526+
skirmishMapSelectLayout->destroyWindows();
527+
deleteInstance(skirmishMapSelectLayout);
528+
skirmishMapSelectLayout = NULL;
529+
}
530+
527531
skirmishPositionStartSpots();
528532
//TheShell->pop();
529533
//do you need this ??
@@ -583,9 +587,12 @@ WindowMsgHandledType SkirmishMapSelectMenuSystem( GameWindow *window, UnsignedIn
583587
skirmishPositionStartSpots();
584588
skirmishUpdateSlotList();
585589

586-
skirmishMapSelectLayout->destroyWindows();
587-
deleteInstance(skirmishMapSelectLayout);
588-
skirmishMapSelectLayout = NULL;
590+
if (skirmishMapSelectLayout)
591+
{
592+
skirmishMapSelectLayout->destroyWindows();
593+
deleteInstance(skirmishMapSelectLayout);
594+
skirmishMapSelectLayout = NULL;
595+
}
589596
//TheShell->pop();
590597

591598
} // end if

‎Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLMapSelectMenu.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,13 @@ WindowMsgHandledType WOLMapSelectMenuSystem( GameWindow *window, UnsignedInt msg
381381
{
382382
showGameSpyGameOptionsUnderlyingGUIElements( TRUE );
383383

384-
WOLMapSelectLayout->destroyWindows();
385-
deleteInstance(WOLMapSelectLayout);
386-
WOLMapSelectLayout = NULL;
384+
if (WOLMapSelectLayout)
385+
{
386+
WOLMapSelectLayout->destroyWindows();
387+
deleteInstance(WOLMapSelectLayout);
388+
WOLMapSelectLayout = NULL;
389+
}
390+
387391
WOLPositionStartSpots();
388392
} // end if
389393
else if ( controlID == radioButtonSystemMapsID )
@@ -445,9 +449,12 @@ WindowMsgHandledType WOLMapSelectMenuSystem( GameWindow *window, UnsignedInt msg
445449
WOLDisplaySlotList();
446450
WOLDisplayGameOptions();
447451

448-
WOLMapSelectLayout->destroyWindows();
449-
deleteInstance(WOLMapSelectLayout);
450-
WOLMapSelectLayout = NULL;
452+
if (WOLMapSelectLayout)
453+
{
454+
WOLMapSelectLayout->destroyWindows();
455+
deleteInstance(WOLMapSelectLayout);
456+
WOLMapSelectLayout = NULL;
457+
}
451458

452459
showGameSpyGameOptionsUnderlyingGUIElements( TRUE );
453460

‎Generals/Code/GameEngine/Source/GameClient/GUI/Shell/Shell.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -629,15 +629,18 @@ void Shell::doPop( Bool impendingPush )
629629

630630
// there better be a top of the stack since we're popping
631631
DEBUG_ASSERTCRASH( currentTop, ("Shell: No top of stack and we want to pop!\n") );
632-
633-
// remove this screen from our list
634-
unlinkScreen( currentTop );
635632

636-
// delete all the windows in the screen
637-
currentTop->destroyWindows();
633+
if (currentTop)
634+
{
635+
// remove this screen from our list
636+
unlinkScreen(currentTop);
637+
638+
// delete all the windows in the screen
639+
currentTop->destroyWindows();
638640

639-
// release the screen object back to the memory pool
640-
deleteInstance(currentTop);
641+
// release the screen object back to the memory pool
642+
deleteInstance(currentTop);
643+
}
641644

642645
// run the init for the new top of the stack if present
643646
WindowLayout *newTop = top();

‎Generals/Code/GameEngine/Source/GameClient/GUI/WindowLayout.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,6 @@ void WindowLayout::removeWindow( GameWindow *window )
173173
//-------------------------------------------------------------------------------------------------
174174
void WindowLayout::destroyWindows( void )
175175
{
176-
if (this == NULL)
177-
{
178-
return;
179-
}
180176
GameWindow *window;
181177

182178
while( (window = getFirstWindow()) != 0 )

‎GeneralsMD/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,8 +977,8 @@ ControlBar::~ControlBar( void )
977977
{
978978
m_scienceLayout->destroyWindows();
979979
deleteInstance(m_scienceLayout);
980+
m_scienceLayout = NULL;
980981
}
981-
m_scienceLayout = NULL;
982982
m_genArrow = NULL;
983983
if(m_videoManager)
984984
delete m_videoManager;

‎GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Diplomacy.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,8 @@ void ResetDiplomacy( void )
292292
theLayout->destroyWindows();
293293
deleteInstance(theLayout);
294294
InitBuddyControls(-1);
295+
theLayout = NULL;
295296
}
296-
theLayout = NULL;
297297
theWindow = NULL;
298298
if (theAnimateWindowManager)
299299
delete theAnimateWindowManager;

‎GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/DifficultySelect.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,12 @@ WindowMsgHandledType DifficultySelectSystem( GameWindow *window, UnsignedInt msg
258258
pref.write();
259259
//TheScriptEngine->setGlobalDifficulty(s_AIDiff); // CANNOT DO THIS! REPLAYS WILL BREAK!
260260
WindowLayout *layout = window->winGetLayout();
261-
layout->destroyWindows();
262-
deleteInstance(layout);
261+
if (layout)
262+
{
263+
layout->destroyWindows();
264+
deleteInstance(layout);
265+
}
266+
263267
setupGameStart(TheCampaignManager->getCurrentMap(), s_AIDiff);
264268
// start the game
265269
}
@@ -268,8 +272,11 @@ WindowMsgHandledType DifficultySelectSystem( GameWindow *window, UnsignedInt msg
268272
TheCampaignManager->setCampaign( AsciiString::TheEmptyString );
269273
TheWindowManager->winUnsetModal(window);
270274
WindowLayout *layout = window->winGetLayout();
271-
layout->destroyWindows();
272-
deleteInstance(layout);
275+
if (layout)
276+
{
277+
layout->destroyWindows();
278+
deleteInstance(layout);
279+
}
273280

274281
}
275282
else if ( controlID == radioButtonEasyAIID )

‎GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/DownloadMenu.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,14 @@ static void closeDownloadWindow( void )
8282
if (!parent)
8383
return;
8484

85-
WindowLayout *menuLayout = parent->winGetLayout();
86-
menuLayout->runShutdown();
87-
menuLayout->destroyWindows();
88-
deleteInstance(menuLayout);
89-
menuLayout = NULL;
85+
WindowLayout *menuLayout = parent->winGetLayout();
86+
if (menuLayout)
87+
{
88+
menuLayout->runShutdown();
89+
menuLayout->destroyWindows();
90+
deleteInstance(menuLayout);
91+
menuLayout = NULL;
92+
}
9093

9194
GameWindow *mainWin = TheWindowManager->winGetWindowFromId( NULL, NAMEKEY("MainMenu.wnd:MainMenuParent") );
9295
if (mainWin)

‎GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/LanMapSelectMenu.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,13 @@ WindowMsgHandledType LanMapSelectMenuSystem( GameWindow *window, UnsignedInt msg
344344
else if ( controlID == buttonBack )
345345
{
346346

347-
mapSelectLayout->destroyWindows();
348-
deleteInstance(mapSelectLayout);
349-
mapSelectLayout = NULL;
347+
if (mapSelectLayout)
348+
{
349+
mapSelectLayout->destroyWindows();
350+
deleteInstance(mapSelectLayout);
351+
mapSelectLayout = NULL;
352+
}
353+
350354
// set the controls to NULL since they've been destroyed.
351355
NullifyControls();
352356
showLANGameOptionsUnderlyingGUIElements(TRUE);
@@ -390,9 +394,12 @@ WindowMsgHandledType LanMapSelectMenuSystem( GameWindow *window, UnsignedInt msg
390394
}
391395

392396

393-
mapSelectLayout->destroyWindows();
394-
deleteInstance(mapSelectLayout);
395-
mapSelectLayout = NULL;
397+
if (mapSelectLayout)
398+
{
399+
mapSelectLayout->destroyWindows();
400+
deleteInstance(mapSelectLayout);
401+
mapSelectLayout = NULL;
402+
}
396403

397404
// set the controls to NULL since they've been destroyed.
398405
NullifyControls();

‎GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupCommunicator.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,13 @@ WindowMsgHandledType PopupCommunicatorSystem( GameWindow *window, UnsignedInt ms
184184

185185
if( controlID == buttonOkID )
186186
{
187-
WindowLayout *popupCommunicatorLayout = window->winGetLayout();
188-
popupCommunicatorLayout->destroyWindows();
189-
deleteInstance(popupCommunicatorLayout);
190-
popupCommunicatorLayout = NULL;
187+
WindowLayout *popupCommunicatorLayout = window->winGetLayout();
188+
if (popupCommunicatorLayout)
189+
{
190+
popupCommunicatorLayout->destroyWindows();
191+
deleteInstance(popupCommunicatorLayout);
192+
popupCommunicatorLayout = NULL;
193+
}
191194
} // end if
192195

193196
break;

‎GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/QuitMenu.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,14 @@ void destroyQuitMenu()
122122
{
123123
fullQuitMenuLayout->destroyWindows();
124124
deleteInstance(fullQuitMenuLayout);
125+
fullQuitMenuLayout = NULL;
125126
}
126-
fullQuitMenuLayout = NULL;
127127
if(noSaveLoadQuitMenuLayout)
128128
{
129129
noSaveLoadQuitMenuLayout->destroyWindows();
130130
deleteInstance(noSaveLoadQuitMenuLayout);
131+
noSaveLoadQuitMenuLayout = NULL;
131132
}
132-
noSaveLoadQuitMenuLayout = NULL;
133133
quitMenuLayout = NULL;
134134
isVisible = FALSE;
135135
}

0 commit comments

Comments
 (0)