Don't try to get the address of the zeroth element in an empty array

It happens to behave nicely with MSVC's RelWithDebInfo, but it's undefined behaviour, and GCC (I don't know what parameters were used) errors due to a bounds-check assertion failure.
This commit is contained in:
Oliver Hamlet
2021-10-12 21:11:25 +01:00
parent e8414a12cf
commit b74da11fc6
7 changed files with 109 additions and 21 deletions
+2 -2
View File
@@ -191,7 +191,7 @@ void Game::LoadPlugins(const std::vector<std::string>& plugins,
thread.join();
}
conditionEvaluator_->RefreshState(cache_);
conditionEvaluator_->RefreshLoadedPluginsState(GetLoadedPlugins());
}
std::shared_ptr<const PluginInterface> Game::GetPlugin(
@@ -224,7 +224,7 @@ std::vector<std::string> Game::SortPlugins(
void Game::LoadCurrentLoadOrderState() {
loadOrderHandler_->LoadCurrentState();
conditionEvaluator_->RefreshState(loadOrderHandler_);
conditionEvaluator_->RefreshActivePluginsState(loadOrderHandler_->GetActivePlugins());
}
bool Game::IsPluginActive(const std::string& pluginName) const {
+15
View File
@@ -127,6 +127,11 @@ std::optional<std::string> ExtractVersion(const std::string& text) {
#ifdef _WIN32
std::wstring ToWinWide(const std::string& str) {
size_t len = MultiByteToWideChar(CP_UTF8, 0, str.c_str(), str.length(), 0, 0);
if (len == 0) {
return std::wstring();
}
std::wstring wstr(len, 0);
MultiByteToWideChar(CP_UTF8, 0, str.c_str(), str.length(), &wstr[0], len);
return wstr;
@@ -135,6 +140,11 @@ std::wstring ToWinWide(const std::string& str) {
std::string FromWinWide(const std::wstring& wstr) {
size_t len = WideCharToMultiByte(
CP_UTF8, 0, wstr.c_str(), wstr.length(), NULL, 0, NULL, NULL);
if (len == 0) {
return std::string();
}
std::string str(len, 0);
WideCharToMultiByte(
CP_UTF8, 0, wstr.c_str(), wstr.length(), &str[0], len, NULL, NULL);
@@ -170,6 +180,11 @@ int CompareFilenames(const std::string& lhs, const std::string& rhs) {
std::string NormalizeFilename(const std::string& filename) {
#ifdef _WIN32
auto wideString = ToWinWide(filename);
if (wideString.empty()) {
return std::string();
}
CharUpperBuffW(&wideString[0], wideString.length());
return FromWinWide(wideString);
#else
+32 -14
View File
@@ -186,28 +186,34 @@ void ConditionEvaluator::ClearConditionCache() {
HandleError("clear the condition cache", result);
}
void ConditionEvaluator::RefreshState(std::shared_ptr<LoadOrderHandler> loadOrderHandler) {
void ConditionEvaluator::RefreshActivePluginsState(
std::vector<std::string> activePluginNames) {
ClearConditionCache();
std::vector<std::string> activePluginNameStrings = loadOrderHandler->GetActivePlugins();
std::vector<const char *> activePluginNames;
for (auto& pluginName : activePluginNameStrings) {
activePluginNames.push_back(pluginName.c_str());
std::vector<const char *> activePluginNameCStrings;
for (auto& pluginName : activePluginNames) {
activePluginNameCStrings.push_back(pluginName.c_str());
}
int result = lci_state_set_active_plugins(lciState_.get(),
&activePluginNames[0],
activePluginNames.size());
const char* const* cActivePluginNames;
if (activePluginNameCStrings.empty()) {
cActivePluginNames = {};
} else {
cActivePluginNames = &activePluginNameCStrings[0];
}
int result = lci_state_set_active_plugins(lciState_.get(), cActivePluginNames, activePluginNameCStrings.size());
HandleError("cache active plugins for condition evaluation", result);
}
void ConditionEvaluator::RefreshState(std::shared_ptr<GameCache> gameCache) {
void ConditionEvaluator::RefreshLoadedPluginsState(
std::vector<std::shared_ptr<const PluginInterface>> plugins) {
ClearConditionCache();
std::vector<std::string> pluginNames;
std::vector<std::string> pluginVersionStrings;
std::vector<uint32_t> crcs;
for (auto plugin : gameCache->GetPlugins()) {
for (auto plugin : plugins) {
pluginNames.push_back(plugin->GetName());
pluginVersionStrings.push_back(plugin->GetVersion().value_or(""));
crcs.push_back(plugin->GetCRC().value_or(0));
@@ -231,13 +237,25 @@ void ConditionEvaluator::RefreshState(std::shared_ptr<GameCache> gameCache) {
}
}
int result = lci_state_set_plugin_versions(lciState_.get(),
&pluginVersions[0],
const plugin_version* cPluginVersions;
if (pluginVersions.empty()) {
cPluginVersions = {};
} else {
cPluginVersions = &pluginVersions[0];
}
int result = lci_state_set_plugin_versions(lciState_.get(), cPluginVersions,
pluginVersions.size());
HandleError("cache plugin versions for condition evaluation", result);
result = lci_state_set_crc_cache(lciState_.get(),
&pluginCrcs[0],
const plugin_crc* cPluginCrcs;
if (pluginCrcs.empty()) {
cPluginCrcs = {};
} else {
cPluginCrcs = &pluginCrcs[0];
}
result = lci_state_set_crc_cache(lciState_.get(), cPluginCrcs,
pluginCrcs.size());
HandleError("fill CRC cache for condition evaluation", result);
}
+3 -2
View File
@@ -45,8 +45,9 @@ public:
PluginMetadata EvaluateAll(const PluginMetadata& pluginMetadata);
void ClearConditionCache();
void RefreshState(std::shared_ptr<LoadOrderHandler> loadOrderHandler);
void RefreshState(std::shared_ptr<GameCache> gameCache);
void RefreshActivePluginsState(std::vector<std::string> activePluginNames);
void RefreshLoadedPluginsState(std::vector<std::shared_ptr<const PluginInterface>> plugins);
private:
bool Evaluate(const PluginCleaningData& cleaningData,
const std::string& pluginName);
+1 -1
View File
@@ -64,7 +64,7 @@ struct convert<loot::Tag> {
} else
tag = node.as<std::string>();
if (tag[0] == '-')
if (!tag.empty() && tag[0] == '-')
rhs = loot::Tag(tag.substr(1), false, condition);
else
rhs = loot::Tag(tag, true, condition);
@@ -288,6 +288,11 @@ TEST(NormalizeFilename, shouldUppercaseStringsAndBeLocaleInvariant) {
// Reset locale.
std::locale::global(std::locale::classic());
}
TEST(NormalizeFilename, shouldReturnAnEmptyStringIfGivenAnEmptyString) {
EXPECT_EQ("", NormalizeFilename(std::string()));
EXPECT_EQ("", NormalizeFilename(""));
}
#else
TEST(NormalizeFilename, shouldCaseFoldStringsAndBeLocaleInvariant) {
// ICU folds all greek rhos to the lowercase rho, unlike Windows. The result
@@ -54,8 +54,9 @@ protected:
out.close();
loadInstalledPlugins();
evaluator_.RefreshState(game_.GetCache());
evaluator_.RefreshState(game_.GetLoadOrderHandler());
evaluator_.RefreshLoadedPluginsState(game_.GetLoadedPlugins());
evaluator_.RefreshActivePluginsState(
game_.GetLoadOrderHandler()->GetActivePlugins());
}
std::string IntToHexString(const uint32_t value) {
@@ -206,6 +207,54 @@ TEST_P(ConditionEvaluatorTest, evaluateAllShouldPreserveGroupExplicitness) {
EXPECT_NO_THROW(plugin = evaluator_.EvaluateAll(plugin));
EXPECT_FALSE(plugin.GetGroup());
}
TEST_P(ConditionEvaluatorTest,
refreshActivePluginsStateShouldClearTheConditionCache) {
std::string condition("active(\"" + blankEsm + "\")");
ASSERT_TRUE(evaluator_.Evaluate(condition));
evaluator_.RefreshActivePluginsState({blankEsp});
EXPECT_FALSE(evaluator_.Evaluate(condition));
}
TEST_P(ConditionEvaluatorTest,
refreshActivePluginsStateShouldClearTheActivePluginsCacheIfGivenAnEmptyVector) {
std::string condition("active(\"" + blankEsm + "\")");
ASSERT_TRUE(evaluator_.Evaluate(condition));
evaluator_.RefreshActivePluginsState({});
EXPECT_FALSE(evaluator_.Evaluate(condition));
}
TEST_P(ConditionEvaluatorTest,
refreshLoadedPluginsStateShouldClearTheConditionCache) {
std::string condition("version(\"" + blankEsm + "\", \"5.0\", ==)");
ASSERT_TRUE(evaluator_.Evaluate(condition));
auto plugins = game_.GetLoadedPlugins();
auto pluginsIt =
std::find_if(plugins.cbegin(), plugins.cend(), [&](auto plugin) {
return plugin->GetName() == blankEsm;
});
plugins.erase(pluginsIt);
evaluator_.RefreshLoadedPluginsState(plugins);
EXPECT_FALSE(evaluator_.Evaluate(condition));
}
TEST_P(ConditionEvaluatorTest,
refreshLoadedPluginsStateShouldClearTheVersionsCacheIfGivenAnEmptyVector) {
std::string condition("version(\"" + blankEsm + "\", \"5.0\", ==)");
ASSERT_TRUE(evaluator_.Evaluate(condition));
evaluator_.RefreshLoadedPluginsState({});
EXPECT_FALSE(evaluator_.Evaluate(condition));
}
}
}