Skip to content

Commit 278a4bb

Browse files
sawenzelsawenzel
committed
Stability fixes in ConfigurableParam
- Prevent problems caused by empty value strings. - Use const& - tighter exception catching
1 parent 13ae7bc commit 278a4bb

2 files changed

Lines changed: 53 additions & 25 deletions

File tree

‎Common/Utils/include/CommonUtils/ConfigurableParam.h‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -180,34 +180,42 @@ class ConfigurableParam
180180
template <typename T>
181181
static void setValue(std::string const& mainkey, std::string const& subkey, T x)
182182
{
183-
auto key = mainkey + "." + subkey;
184-
if (sPtree->get_optional<std::string>(key).is_initialized()) {
185-
sPtree->put(key, x);
186-
auto changed = updateThroughStorageMap(mainkey, subkey, typeid(T), (void*)&x);
187-
if (changed) {
188-
sValueProvenanceMap->find(key)->second = kRT; // set to runtime
183+
try {
184+
auto key = mainkey + "." + subkey;
185+
if (sPtree->get_optional<std::string>(key).is_initialized()) {
186+
sPtree->put(key, x);
187+
auto changed = updateThroughStorageMap(mainkey, subkey, typeid(T), (void*)&x);
188+
if (changed) {
189+
sValueProvenanceMap->find(key)->second = kRT; // set to runtime
190+
}
189191
}
192+
} catch (std::exception const& e) {
193+
std::cerr << "Error in setValue (T) " << e.what() << "\n";
190194
}
191195
}
192196

193197
// specialized for std::string
194198
// which means that the type will be converted internally
195199
static void setValue(std::string const& key, std::string const& valuestring)
196200
{
197-
if (sPtree->get_optional<std::string>(key).is_initialized()) {
198-
sPtree->put(key, valuestring);
199-
auto changed = updateThroughStorageMapWithConversion(key, valuestring);
200-
if (changed) {
201-
sValueProvenanceMap->find(key)->second = kRT; // set to runtime
201+
try {
202+
if (sPtree->get_optional<std::string>(key).is_initialized()) {
203+
sPtree->put(key, valuestring);
204+
auto changed = updateThroughStorageMapWithConversion(key, valuestring);
205+
if (changed) {
206+
sValueProvenanceMap->find(key)->second = kRT; // set to runtime
207+
}
202208
}
209+
} catch (std::exception const& e) {
210+
std::cerr << "Error in setValue (string) " << e.what() << "\n";
203211
}
204212
}
205213

206214
static void setEnumValue(const std::string&, const std::string&);
207215
static void setArrayValue(const std::string&, const std::string&);
208216

209217
// update the storagemap from a vector of key/value pairs, calling setValue for each pair
210-
static void setValues(std::vector<std::pair<std::string, std::string>> keyValues);
218+
static void setValues(std::vector<std::pair<std::string, std::string>> const& keyValues);
211219

212220
// initializes the parameter database
213221
static void initialize();

‎Common/Utils/src/ConfigurableParam.cxx‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,16 @@ std::vector<std::string> splitString(const std::string& src, char delim, bool tr
9696
// Does the given key exist in the boost property tree?
9797
bool keyInTree(boost::property_tree::ptree* pt, std::string key)
9898
{
99-
return pt->get_optional<std::string>(key).is_initialized();
99+
if (key.size() == 0 || pt == nullptr) {
100+
return false;
101+
}
102+
bool reply = false;
103+
try {
104+
reply = pt->get_optional<std::string>(key).is_initialized();
105+
} catch (std::exception const& e) {
106+
LOG(ERROR) << "ConfigurableParam: Exception when checking for key " << key << " : " << e.what();
107+
}
108+
return reply;
100109
}
101110

102111
// ------------------------------------------------------------------
@@ -237,6 +246,8 @@ boost::property_tree::ptree ConfigurableParam::readINI(std::string const& filepa
237246
boost::property_tree::read_ini(filepath, pt);
238247
} catch (const boost::property_tree::ptree_error& e) {
239248
LOG(FATAL) << "Failed to read INI config file " << filepath << " (" << e.what() << ")";
249+
} catch (...) {
250+
LOG(FATAL) << "Unknown error when reading INI config file ";
240251
}
241252

242253
return pt;
@@ -397,19 +408,28 @@ void ConfigurableParam::updateFromFile(std::string const& configFile)
397408

398409
std::vector<std::pair<std::string, std::string>> keyValPairs;
399410

400-
for (auto& section : pt) {
401-
std::string mainKey = section.first;
402-
for (auto& subKey : section.second) {
403-
auto name = subKey.first;
404-
auto value = subKey.second.get_value<std::string>();
405-
std::string key = mainKey + "." + name;
406-
407-
std::pair<std::string, std::string> pair = std::make_pair(key, trimSpace(value));
408-
keyValPairs.push_back(pair);
411+
try {
412+
for (auto& section : pt) {
413+
std::string mainKey = section.first;
414+
for (auto& subKey : section.second) {
415+
auto name = subKey.first;
416+
auto value = subKey.second.get_value<std::string>();
417+
std::string key = mainKey + "." + name;
418+
std::pair<std::string, std::string> pair = std::make_pair(key, trimSpace(value));
419+
keyValPairs.push_back(pair);
420+
}
409421
}
422+
} catch (std::exception const& error) {
423+
LOG(ERROR) << "Error while updating params " << error.what();
424+
} catch (...) {
425+
LOG(ERROR) << "Unknown while updating params ";
410426
}
411427

412-
setValues(keyValPairs);
428+
try {
429+
setValues(keyValPairs);
430+
} catch (std::exception const& error) {
431+
LOG(ERROR) << "Error while setting values " << error.what();
432+
}
413433
}
414434

415435
// ------------------------------------------------------------------
@@ -475,10 +495,10 @@ void ConfigurableParam::updateFromString(std::string const& configString)
475495

476496
// setValues takes a vector of pairs where each pair is a key and value
477497
// to be set in the storage map
478-
void ConfigurableParam::setValues(std::vector<std::pair<std::string, std::string>> keyValues)
498+
void ConfigurableParam::setValues(std::vector<std::pair<std::string, std::string>> const& keyValues)
479499
{
480500
auto isArray = [](std::string& el) {
481-
return (el.at(0) == '[') && (el.at(el.size() - 1) == ']');
501+
return el.size() > 0 && (el.at(0) == '[') && (el.at(el.size() - 1) == ']');
482502
};
483503

484504
// Take a vector of param key/value pairs

0 commit comments

Comments
 (0)