Browse Source

client/lobby/SelectionTab.cpp: initialize generalSortingBy before use

Noticed use of uninitialized value when built vcmi with -fsanitize=undefined:

```
  Established connection with
    VCMI 0.99 b310f2e61ece1ab550614cf1b99b04830820700c (server).
    UUID: bab9a90d-7416-4566-8817-e367ffcac7c1
  ../vcmi-9999/client/lobby/SelectionTab.cpp:138:16:
    runtime error: load of value 32717, which is not a valid value for type 'ESortBy'
  /usr/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/g++-v12/tuple:190:4:
    runtime error: load of value 32717, which is not a valid value for type 'ESortBy'
  /usr/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/g++-v12/tuple:190:4:
    runtime error: load of value 32717, which is not a valid value for type 'ESortBy'
```

Here (before the change) `SelectionTab()` used `generalSortingBy` before
first use:

```
    SelectionTab::SelectionTab(ESelectionScreen Type) {
        ...
        if(tabType != ESelectionScreen::campaignList)
        {
            ...
                ESortBy criteria = (ESortBy)i;
                if(criteria == _name)
                    criteria = generalSortingBy;

                buttonsSortBy.push_back(... std::bind(&SelectionTab::sortBy, this, criteria)));
            ...
        }
        ...
        switch(tabType)
        {
        case ESelectionScreen::newGame:
                generalSortingBy = ESortBy::_name;
        ....
```

The change moves `generalSortingBy` initialization before first use.

Signed-off-by: Sergei Trofimovich <[email protected]>
Sergei Trofimovich 4 years ago
parent
commit
e4dd2f6dd8
1 changed files with 20 additions and 4 deletions
  1. 20 4
      client/lobby/SelectionTab.cpp

+ 20 - 4
client/lobby/SelectionTab.cpp

@@ -111,10 +111,30 @@ bool mapSorter::operator()(const std::shared_ptr<CMapInfo> aaa, const std::share
 	}
 }
 
+// pick sorting order based on selection
+static ESortBy getSortBySelectionScreen(ESelectionScreen Type)
+{
+	switch(Type)
+	{
+	case ESelectionScreen::newGame:
+		return ESortBy::_name;
+	case ESelectionScreen::loadGame:
+	case ESelectionScreen::saveGame:
+		return ESortBy::_fileName;
+	case ESelectionScreen::campaignList:
+		return ESortBy::_name;
+	}
+	// Should not reach here. But let's not crash the game.
+	return ESortBy::_name;
+}
+
 SelectionTab::SelectionTab(ESelectionScreen Type)
 	: CIntObject(LCLICK | WHEEL | KEYBOARD | DOUBLECLICK), callOnSelect(nullptr), tabType(Type), selectionPos(0), sortModeAscending(true)
 {
 	OBJ_CONSTRUCTION;
+
+	generalSortingBy = getSortBySelectionScreen(tabType);
+
 	if(tabType != ESelectionScreen::campaignList)
 	{
 		sortingBy = _format;
@@ -146,20 +166,16 @@ SelectionTab::SelectionTab(ESelectionScreen Type)
 	switch(tabType)
 	{
 	case ESelectionScreen::newGame:
-		generalSortingBy = ESortBy::_name;
 		tabTitle = CGI->generaltexth->arraytxt[229];
 		break;
 	case ESelectionScreen::loadGame:
-		generalSortingBy = ESortBy::_fileName;
 		tabTitle = CGI->generaltexth->arraytxt[230];
 		break;
 	case ESelectionScreen::saveGame:
 		positionsToShow = 16;
-		generalSortingBy = ESortBy::_fileName;
 		tabTitle = CGI->generaltexth->arraytxt[231];
 		break;
 	case ESelectionScreen::campaignList:
-		generalSortingBy = ESortBy::_name;
 		tabTitle = CGI->generaltexth->allTexts[726];
 		type |= REDRAW_PARENT; // we use parent background so we need to make sure it's will be redrawn too
 		pos.w = parent->pos.w;