Skip to content

Commit

Permalink
Fix memory leak in single-player difficulty selection
Browse files Browse the repository at this point in the history
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
    #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
    #10 0x556776a44f45 in mainmenu_select_hero_dialog ../Source/mainmenu.cpp:98
    #11 0x5567765f9f15 in SNetInitializeProvider ../SourceX/storm/storm_net.cpp:102
    #12 0x556776c996b9 in multi_init_single ../Source/multi.cpp:826
    #13 0x556776c98b1e in NetInit ../Source/multi.cpp:770
    #14 0x5567767d0c0b in StartGame ../Source/diablo.cpp:375
    #15 0x556776a4493d in mainmenu_init_menu ../Source/mainmenu.cpp:45
    #16 0x556776a44c05 in mainmenu_single_player ../Source/mainmenu.cpp:61
    #17 0x556776a454a9 in mainmenu_loop ../Source/mainmenu.cpp:152
    #18 0x5567767d2892 in DiabloMain ../Source/diablo.cpp:602
    #19 0x5567766e3c69 in main ../SourceX/main.cpp:34
    #20 0x7f435a69ecb1 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28cb1)
```
  • Loading branch information
glebm authored and AJenbo committed Mar 23, 2021
1 parent 58f7ad1 commit 7269ade
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
14 changes: 14 additions & 0 deletions SourceX/DiabloUI/selgame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,21 @@ void selgame_Diff_Select(int value)
nDifficulty = value;

if (!selhero_isMultiPlayer) {
// This is part of a dangerous hack to enable difficulty selection in single-player.
// FIXME: Dialogs should not refer to each other's variables.

// We're in the selhero loop instead of the selgame one.
// Free the selgame data and flag the end of the selhero loop.
selhero_endMenu = true;

// We only call FreeVectors because ArtBackground.Unload()
// will be called by selheroFree().
selgame_FreeVectors();

// We must clear the InitList because selhero's loop will perform
// one more iteration after this function exits.
UiInitList_clear();

return;
}

Expand Down
15 changes: 12 additions & 3 deletions SourceX/DiabloUI/selhero.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ void selhero_Free()

selhero_FreeDlgItems();
selhero_FreeListItems();
UnloadScrollBar();

bUIElementsLoaded = false;
}
Expand Down Expand Up @@ -457,7 +458,17 @@ void selhero_Load_Select(int value)
if (vecSelHeroDlgItems[value]->m_value == 0) {
selhero_result = SELHERO_CONTINUE;
return;
} else if (!selhero_isMultiPlayer) {
}

if (!selhero_isMultiPlayer) {
// This is part of a dangerous hack to enable difficulty selection in single-player.
// FIXME: Dialogs should not refer to each other's variables.

// We disable `selhero_endMenu` and replace the background and art
// and the item list with the difficulty selection ones.
//
// This means selhero's render loop will render selgame's items,
// which happens to work because the render loops are similar.
selhero_endMenu = false;
selhero_Free();
LoadBackgroundArt("ui_art\\selgame.pcx");
Expand Down Expand Up @@ -530,8 +541,6 @@ static void UiSelHeroDialog(

*dlgresult = selhero_result;
snprintf(*name, sizeof(*name), selhero_heroInfo.name);

UnloadScrollBar();
}

void UiSelHeroSingDialog(
Expand Down

0 comments on commit 7269ade

Please sign in to comment.