andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix an issue with Configurable::AreEquivalent

Browse Source 
If the registered options did not have names, bad things could happen.
This commit is contained in:
mrambacher 2022-05-17 09:20:21 -04:00
parent c4cd8e1acc
commit f99aef6bd9
3 changed files with 60 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
options/configurable.cc
View File

@ -690,14 +690,26 @@ bool ConfigurableHelper::AreEquivalent(const ConfigOptions& config_options,
const Configurable& that_one,
std::string* mismatch) {
assert(mismatch != nullptr);
for (auto const& o : this_one.options_) {
const auto this_offset = this_one.GetOptionsPtr(o.name);
const auto that_offset = that_one.GetOptionsPtr(o.name);
if (this_one.options_.size() != that_one.options_.size()) {
// The two types do not have the same number of registered options,
// therefore they cannot be the same.
return false;
}
for (size_t i = 0; i < this_one.options_.size(); i++) {
const auto& this_opt = this_one.options_[i];
const auto& that_opt = that_one.options_[i];
if (this_opt.name != that_opt.name ||
this_opt.type_map != that_opt.type_map) {
return false;
}
const auto this_offset = this_opt.opt_ptr;
const auto that_offset = that_opt.opt_ptr;
if (this_offset != that_offset) {
if (this_offset == nullptr || that_offset == nullptr) {
return false;
} else if (o.type_map != nullptr) {
for (const auto& map_iter : *(o.type_map)) {
} else if (this_opt.type_map != nullptr) {
for (const auto& map_iter : *(this_opt.type_map)) {
const auto& opt_info = map_iter.second;
if (config_options.IsCheckEnabled(opt_info.GetSanityLevel())) {
if (!config_options.mutable_options_only) {

32
options/configurable_test.cc
View File

@ -643,17 +643,45 @@ TEST_F(ConfigurableTest, TestNoCompare) {
std::unique_ptr<Configurable> base, copy;
base.reset(SimpleConfigurable::Create("c", TestConfigMode::kDefaultMode,
&nocomp_option_info));
&normal_option_info));
copy.reset(SimpleConfigurable::Create("c", TestConfigMode::kDefaultMode,
&normal_option_info));
ASSERT_OK(base->ConfigureFromString(config_options_, "int=10"));
ASSERT_OK(copy->ConfigureFromString(config_options_, "int=20"));
ASSERT_OK(copy->ConfigureFromString(config_options_, "int=10"));
std::string bvalue, cvalue, mismatch;
ASSERT_OK(base->GetOption(config_options_, "int", &bvalue));
ASSERT_OK(copy->GetOption(config_options_, "int", &cvalue));
ASSERT_EQ(bvalue, "10");
ASSERT_EQ(cvalue, "10");
ASSERT_TRUE(base->AreEquivalent(config_options_, copy.get(), &mismatch));
ASSERT_TRUE(copy->AreEquivalent(config_options_, base.get(), &mismatch));
ASSERT_OK(copy->ConfigureFromString(config_options_, "int=20"));
ASSERT_OK(copy->GetOption(config_options_, "int", &cvalue));
ASSERT_EQ(cvalue, "20");
ASSERT_FALSE(base->AreEquivalent(config_options_, copy.get(), &mismatch));
ASSERT_FALSE(copy->AreEquivalent(config_options_, base.get(), &mismatch));
base.reset(SimpleConfigurable::Create("c", TestConfigMode::kDefaultMode,
&nocomp_option_info));
copy.reset(SimpleConfigurable::Create("c", TestConfigMode::kDefaultMode,
&nocomp_option_info));
ASSERT_OK(base->ConfigureFromString(config_options_, "int=10"));
ASSERT_OK(copy->ConfigureFromString(config_options_, "int=20"));
ASSERT_OK(base->GetOption(config_options_, "int", &bvalue));
ASSERT_OK(copy->GetOption(config_options_, "int", &cvalue));
ASSERT_EQ(bvalue, "10");
ASSERT_EQ(cvalue, "20");
ASSERT_TRUE(base->AreEquivalent(config_options_, copy.get(), &mismatch));
ASSERT_TRUE(copy->AreEquivalent(config_options_, base.get(), &mismatch));
copy.reset(SimpleConfigurable::Create("c", TestConfigMode::kDefaultMode,
&normal_option_info));
ASSERT_OK(copy->ConfigureFromString(config_options_, "int=10"));
ASSERT_OK(copy->GetOption(config_options_, "int", &cvalue));
ASSERT_EQ(bvalue, cvalue);
// The registered options do not match. The values will not match
ASSERT_FALSE(base->AreEquivalent(config_options_, copy.get(), &mismatch));
ASSERT_FALSE(copy->AreEquivalent(config_options_, base.get(), &mismatch));
}

13
options/customizable_test.cc
View File

@ -376,6 +376,19 @@ TEST_F(CustomizableTest, SimpleConfigureTest) {
configurable->AreEquivalent(config_options_, copy.get(), &mismatch));
}
TEST_F(CustomizableTest, AreEquivalentTest) {
std::string mismatch;
std::unique_ptr<Customizable> base(new ACustomizable("A"));
std::unique_ptr<Customizable> copy(new ACustomizable("A"));
ASSERT_TRUE(base->AreEquivalent(config_options_, copy.get(), &mismatch));
copy.reset(new ACustomizable("B"));
ASSERT_FALSE(base->AreEquivalent(config_options_, copy.get(), &mismatch));
ASSERT_FALSE(copy->AreEquivalent(config_options_, base.get(), &mismatch));
copy.reset(new TestCustomizable("A"));
ASSERT_FALSE(base->AreEquivalent(config_options_, copy.get(), &mismatch));
ASSERT_FALSE(copy->AreEquivalent(config_options_, base.get(), &mismatch));
}
TEST_F(CustomizableTest, ConfigureFromPropsTest) {
std::unordered_map<std::string, std::string> opt_map = {
{"unique.id", "A"}, {"unique.A.int", "1"}, {"unique.A.bool", "true"},