Skip to content

Commit f7f1426

Browse files
committed
Fix memory leak in single-player difficulty selection
Single-player difficulty selection was implemented in a very hacky way. Documents what's going on there and fixes a memory leak. Memory leaks that this fixes looked like this: ``` Direct leak of 64 byte(s) in 1 object(s) allocated from: #0 0x7f435b789f17 in operator new(unsigned long) (/lib/x86_64-linux-gnu/libasan.so.6+0xb1f17) #1 0x5567766c62b8 in dvl::selgame_GameSelection_Select(int) ../SourceX/DiabloUI/selgame.cpp:163 #2 0x5567766d8ee8 in dvl::selhero_Load_Select(int) ../SourceX/DiabloUI/selhero.cpp:464 #3 0x5567766d6ed1 in dvl::selhero_List_Select(int) ../SourceX/DiabloUI/selhero.cpp:325 #4 0x556776692683 in dvl::UiFocusNavigationSelect() ../SourceX/DiabloUI/diabloui.cpp:396 #5 0x55677669158e in HandleMenuAction ../SourceX/DiabloUI/diabloui.cpp:223 #6 0x5567766917b9 in dvl::UiFocusNavigation(SDL_Event*) ../SourceX/DiabloUI/diabloui.cpp:277 #7 0x556776695dff in dvl::UiPollAndRender() ../SourceX/DiabloUI/diabloui.cpp:626 #8 0x5567766d94ef in UiSelHeroDialog ../SourceX/DiabloUI/selhero.cpp:512 diasurgical#9 0x5567766d997f in dvl::UiSelHeroSingDialog(int (*)(int (*)(dvl::_uiheroinfo*)), int (*)(dvl::_uiheroinfo*), int (*)(dvl::_uiheroinfo*), void (*)(unsigned int, dvl::_uidefaultstats*), int*, char (*) [16], int*) ../SourceX/DiabloUI/selhero.cpp:547 diasurgical#10 0x556776a44f45 in mainmenu_select_hero_dialog ../Source/mainmenu.cpp:98 diasurgical#11 0x5567765f9f15 in SNetInitializeProvider ../SourceX/storm/storm_net.cpp:102 diasurgical#12 0x556776c996b9 in multi_init_single ../Source/multi.cpp:826 diasurgical#13 0x556776c98b1e in NetInit ../Source/multi.cpp:770 diasurgical#14 0x5567767d0c0b in StartGame ../Source/diablo.cpp:375 diasurgical#15 0x556776a4493d in mainmenu_init_menu ../Source/mainmenu.cpp:45 diasurgical#16 0x556776a44c05 in mainmenu_single_player ../Source/mainmenu.cpp:61 diasurgical#17 0x556776a454a9 in mainmenu_loop ../Source/mainmenu.cpp:152 diasurgical#18 0x5567767d2892 in DiabloMain ../Source/diablo.cpp:602 diasurgical#19 0x5567766e3c69 in main ../SourceX/main.cpp:34 diasurgical#20 0x7f435a69ecb1 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28cb1) ```
1 parent a85b03e commit f7f1426

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

SourceX/DiabloUI/selgame.cpp

+14
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,21 @@ void selgame_Diff_Select(int value)
246246
nDifficulty = value;
247247

248248
if (!selhero_isMultiPlayer) {
249+
// This is part of a dangerous hack to enable difficulty selection in single-player.
250+
// FIXME: Dialogs should not refer to each other's variables.
251+
252+
// We're in the selhero loop instead of the selgame one.
253+
// Free the selgame data and flag the end of the selhero loop.
249254
selhero_endMenu = true;
255+
256+
// We only call FreeVectors because ArtBackground.Unload()
257+
// will be called by selheroFree().
258+
selgame_FreeVectors();
259+
260+
// We must clear the InitList because selhero's loop will perform
261+
// one more iteration after this function exits.
262+
UiInitList_clear();
263+
250264
return;
251265
}
252266

SourceX/DiabloUI/selhero.cpp

+12-3
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ void selhero_Free()
8888

8989
selhero_FreeDlgItems();
9090
selhero_FreeListItems();
91+
UnloadScrollBar();
9192

9293
bUIElementsLoaded = false;
9394
}
@@ -457,7 +458,17 @@ void selhero_Load_Select(int value)
457458
if (vecSelHeroDlgItems[value]->m_value == 0) {
458459
selhero_result = SELHERO_CONTINUE;
459460
return;
460-
} else if (!selhero_isMultiPlayer) {
461+
}
462+
463+
if (!selhero_isMultiPlayer) {
464+
// This is part of a dangerous hack to enable difficulty selection in single-player.
465+
// FIXME: Dialogs should not refer to each other's variables.
466+
467+
// We disable `selhero_endMenu` and replace the background and art
468+
// and the item list with the difficulty selection ones.
469+
//
470+
// This means selhero's render loop will render selgame's items,
471+
// which happens to work because the render loops are similar.
461472
selhero_endMenu = false;
462473
selhero_Free();
463474
LoadBackgroundArt("ui_art\\selgame.pcx");
@@ -530,8 +541,6 @@ static void UiSelHeroDialog(
530541

531542
*dlgresult = selhero_result;
532543
snprintf(*name, sizeof(*name), selhero_heroInfo.name);
533-
534-
UnloadScrollBar();
535544
}
536545

537546
void UiSelHeroSingDialog(

0 commit comments

Comments
 (0)