Remove implicit conversions
Implicit conversions are something that nlohmann library itself is a bad
default, and 3 years ago threatened to change the default. These
implicit conversions cause a number of crashes that are hard to
reproduce, because they throw an uncaught exception.
Update the code to be able to do no more implicit conversions.
Tested: Entity-manager launches and runs. Items are detected correctly
and show up on dbus. Unit tests pass.
Change-Id: Ib23159ae58f5584641427d9be7545bc25a3619af
Signed-off-by: Ed Tanous <etanous@nvidia.com>
diff --git a/src/entity_manager/dbus_interface.cpp b/src/entity_manager/dbus_interface.cpp
index 0610812..36ec10d 100644
--- a/src/entity_manager/dbus_interface.cpp
+++ b/src/entity_manager/dbus_interface.cpp
@@ -144,7 +144,7 @@
static void populateInterfacePropertyFromJson(
nlohmann::json& systemConfiguration, const std::string& path,
- const nlohmann::json& key, const nlohmann::json& value,
+ const std::string& key, const nlohmann::json& value,
nlohmann::json::value_t type,
std::shared_ptr<sdbusplus::asio::dbus_interface>& iface,
sdbusplus::asio::PropertyPermission permission)
diff --git a/src/entity_manager/entity_manager.cpp b/src/entity_manager/entity_manager.cpp
index 6bf3c52..d114841 100644
--- a/src/entity_manager/entity_manager.cpp
+++ b/src/entity_manager/entity_manager.cpp
@@ -84,7 +84,15 @@
// iterate through boards
for (const auto& [boardId, boardConfig] : newConfiguration.items())
{
- postBoardToDBus(boardId, boardConfig, newBoards);
+ const nlohmann::json::object_t* boardConfigPtr =
+ boardConfig.get_ptr<const nlohmann::json::object_t*>();
+ if (boardConfigPtr == nullptr)
+ {
+ lg2::error("boardConfig for {BOARD} was not an object", "BOARD",
+ boardId);
+ continue;
+ }
+ postBoardToDBus(boardId, *boardConfigPtr, newBoards);
}
for (const auto& [assocPath, assocPropValue] :
@@ -108,11 +116,24 @@
}
void EntityManager::postBoardToDBus(
- const std::string& boardId, const nlohmann::json& boardConfig,
+ const std::string& boardId, const nlohmann::json::object_t& boardConfig,
std::map<std::string, std::string>& newBoards)
{
- std::string boardName = boardConfig["Name"];
- std::string boardNameOrig = boardConfig["Name"];
+ auto boardNameIt = boardConfig.find("Name");
+ if (boardNameIt == boardConfig.end())
+ {
+ lg2::error("Unable to find name for {BOARD}", "BOARD", boardId);
+ return;
+ }
+ const std::string* boardNamePtr =
+ boardNameIt->second.get_ptr<const std::string*>();
+ if (boardNamePtr == nullptr)
+ {
+ lg2::error("Name for {BOARD} was not a string", "BOARD", boardId);
+ return;
+ }
+ std::string boardName = *boardNamePtr;
+ std::string boardNameOrig = *boardNamePtr;
std::string jsonPointerPath = "/" + boardId;
// loop through newConfiguration, but use values from system
// configuration to be able to modify via dbus later
diff --git a/src/entity_manager/entity_manager.hpp b/src/entity_manager/entity_manager.hpp
index 70f7a4f..aa1c7ea 100644
--- a/src/entity_manager/entity_manager.hpp
+++ b/src/entity_manager/entity_manager.hpp
@@ -53,7 +53,7 @@
nlohmann::json newConfiguration);
void postToDbus(const nlohmann::json& newConfiguration);
void postBoardToDBus(const std::string& boardId,
- const nlohmann::json& boardConfig,
+ const nlohmann::json::object_t& boardConfig,
std::map<std::string, std::string>& newBoards);
void postExposesRecordsToDBus(
nlohmann::json& item, size_t& exposesIndex,
diff --git a/src/entity_manager/perform_scan.cpp b/src/entity_manager/perform_scan.cpp
index 85eb723..c126c0f 100644
--- a/src/entity_manager/perform_scan.cpp
+++ b/src/entity_manager/perform_scan.cpp
@@ -246,17 +246,22 @@
}
static bool extractExposeActionRecordNames(std::vector<std::string>& matches,
- nlohmann::json::iterator& keyPair)
+ const std::string& exposeKey,
+ nlohmann::json& exposeValue)
{
- if (keyPair.value().is_string())
+ const std::string* exposeValueStr =
+ exposeValue.get_ptr<const std::string*>();
+ if (exposeValueStr != nullptr)
{
- matches.emplace_back(keyPair.value());
+ matches.emplace_back(*exposeValueStr);
return true;
}
- if (keyPair.value().is_array())
+ const nlohmann::json::array_t* exarr =
+ exposeValue.get_ptr<const nlohmann::json::array_t*>();
+ if (exarr != nullptr)
{
- for (const auto& value : keyPair.value())
+ for (const auto& value : *exarr)
{
if (!value.is_string())
{
@@ -269,8 +274,7 @@
return true;
}
- lg2::error("Value is invalid type {KEY}", "KEY", keyPair.key());
-
+ lg2::error("Value is invalid type {KEY}", "KEY", exposeKey);
return false;
}
@@ -289,8 +293,8 @@
return matchIt;
}
-static void applyBindExposeAction(nlohmann::json& exposedObject,
- nlohmann::json& expose,
+static void applyBindExposeAction(nlohmann::json::object_t& exposedObject,
+ nlohmann::json::object_t& expose,
const std::string& propertyName)
{
if (propertyName.starts_with("Bind"))
@@ -301,7 +305,7 @@
}
}
-static void applyDisableExposeAction(nlohmann::json& exposedObject,
+static void applyDisableExposeAction(nlohmann::json::object_t& exposedObject,
const std::string& propertyName)
{
if (propertyName == "DisableNode")
@@ -311,8 +315,8 @@
}
static void applyConfigExposeActions(
- std::vector<std::string>& matches, nlohmann::json& expose,
- const std::string& propertyName, nlohmann::json& configExposes)
+ std::vector<std::string>& matches, nlohmann::json::object_t& expose,
+ const std::string& propertyName, nlohmann::json::array_t& configExposes)
{
for (auto& exposedObject : configExposes)
{
@@ -320,18 +324,28 @@
if (match)
{
matches.erase(*match);
- applyBindExposeAction(exposedObject, expose, propertyName);
- applyDisableExposeAction(exposedObject, propertyName);
+ nlohmann::json::object_t* exposedObjectObj =
+ exposedObject.get_ptr<nlohmann::json::object_t*>();
+ if (exposedObjectObj == nullptr)
+ {
+ lg2::error("Exposed object wasn't a object: {JSON}", "JSON",
+ exposedObject.dump());
+ continue;
+ }
+
+ applyBindExposeAction(*exposedObjectObj, expose, propertyName);
+ applyDisableExposeAction(*exposedObjectObj, propertyName);
}
}
}
static void applyExposeActions(
nlohmann::json& systemConfiguration, const std::string& recordName,
- nlohmann::json& expose, nlohmann::json::iterator& keyPair)
+ nlohmann::json::object_t& expose, const std::string& exposeKey,
+ nlohmann::json& exposeValue)
{
- bool isBind = keyPair.key().starts_with("Bind");
- bool isDisable = keyPair.key() == "DisableNode";
+ bool isBind = exposeKey.starts_with("Bind");
+ bool isDisable = exposeKey == "DisableNode";
bool isExposeAction = isBind || isDisable;
if (!isExposeAction)
@@ -341,7 +355,7 @@
std::vector<std::string> matches;
- if (!extractExposeActionRecordNames(matches, keyPair))
+ if (!extractExposeActionRecordNames(matches, exposeKey, exposeValue))
{
return;
}
@@ -360,20 +374,20 @@
continue;
}
- if (!configListFind->is_array())
+ nlohmann::json::array_t* configList =
+ configListFind->get_ptr<nlohmann::json::array_t*>();
+ if (configList == nullptr)
{
continue;
}
-
- applyConfigExposeActions(matches, expose, keyPair.key(),
- *configListFind);
+ applyConfigExposeActions(matches, expose, exposeKey, *configList);
}
if (!matches.empty())
{
lg2::error(
"configuration file dependency error, could not find {KEY} {VALUE}",
- "KEY", keyPair.key(), "VALUE", keyPair.value());
+ "KEY", exposeKey, "VALUE", exposeValue);
}
}
@@ -382,20 +396,17 @@
size_t foundDeviceIdx, const std::string& nameTemplate,
std::optional<std::string>& replaceStr)
{
- nlohmann::json copyForName = {{"Name", nameTemplate}};
- nlohmann::json::iterator copyIt = copyForName.begin();
+ nlohmann::json copyForName = nameTemplate;
std::optional<std::string> replaceVal = em_utils::templateCharReplace(
- copyIt, dbusObject, foundDeviceIdx, replaceStr);
+ copyForName, dbusObject, foundDeviceIdx, replaceStr);
if (!replaceStr && replaceVal)
{
- if (usedNames.contains(copyIt.value()))
+ if (usedNames.contains(nameTemplate))
{
replaceStr = replaceVal;
- copyForName = {{"Name", nameTemplate}};
- copyIt = copyForName.begin();
- em_utils::templateCharReplace(copyIt, dbusObject, foundDeviceIdx,
- replaceStr);
+ em_utils::templateCharReplace(copyForName, dbusObject,
+ foundDeviceIdx, replaceStr);
}
}
@@ -405,9 +416,35 @@
"Duplicates found, replacing {STR} with found device index. Consider fixing template to not have duplicates",
"STR", *replaceStr);
}
-
- return copyIt.value();
+ const std::string* ret = copyForName.get_ptr<const std::string*>();
+ if (ret == nullptr)
+ {
+ lg2::error("Device name wasn't a string: ${JSON}", "JSON",
+ copyForName.dump());
+ return "";
+ }
+ return *ret;
}
+static void applyTemplateAndExposeActions(
+ const std::string& recordName, const DBusObject& dbusObject,
+ size_t foundDeviceIdx, const std::optional<std::string>& replaceStr,
+ nlohmann::json& value, nlohmann::json& systemConfiguration)
+{
+ nlohmann::json::object_t* exposeObj =
+ value.get_ptr<nlohmann::json::object_t*>();
+ if (exposeObj != nullptr)
+ {
+ return;
+ }
+ for (auto& [key, value] : *exposeObj)
+ {
+ em_utils::templateCharReplace(value, dbusObject, foundDeviceIdx,
+ replaceStr);
+
+ applyExposeActions(systemConfiguration, recordName, *exposeObj, key,
+ value);
+ }
+};
void scan::PerformScan::updateSystemConfiguration(
const nlohmann::json& recordRef, const std::string& probeName,
@@ -467,7 +504,15 @@
? emptyObject
: objectIt->second;
- nlohmann::json record = recordRef;
+ const nlohmann::json::object_t* recordPtr =
+ recordRef.get_ptr<const nlohmann::json::object_t*>();
+ if (recordPtr == nullptr)
+ {
+ lg2::error("Failed to parse record {JSON}", "JSON",
+ recordRef.dump());
+ continue;
+ }
+ nlohmann::json::object_t record = *recordPtr;
std::string recordName = getRecordName(foundDevice, probeName);
size_t foundDeviceIdx = indexes.front();
indexes.pop_front();
@@ -476,24 +521,32 @@
auto getName = record.find("Name");
if (getName == record.end())
{
- lg2::error("Record Missing Name! {JSON}", "JSON", record.dump());
+ lg2::error("Record Missing Name! {JSON}", "JSON", recordRef.dump());
continue; // this should be impossible at this level
}
+ const std::string* name = getName->second.get_ptr<const std::string*>();
+ if (name == nullptr)
+ {
+ lg2::error("Name wasn't a string: {JSON}", "JSON",
+ recordRef.dump());
+ continue;
+ }
+
std::string deviceName = generateDeviceName(
- usedNames, dbusObject, foundDeviceIdx, getName.value(), replaceStr);
- getName.value() = deviceName;
+ usedNames, dbusObject, foundDeviceIdx, *name, replaceStr);
+
usedNames.insert(deviceName);
- for (auto keyPair = record.begin(); keyPair != record.end(); keyPair++)
+ for (auto& keyPair : record)
{
- if (keyPair.key() != "Name")
+ if (keyPair.first != "Name")
{
// "Probe" string does not contain template variables
// Handle left-over variables for "Exposes" later below
const bool handleLeftOver =
- (keyPair.key() != "Probe") && (keyPair.key() != "Exposes");
- em_utils::templateCharReplace(keyPair, dbusObject,
+ (keyPair.first != "Probe") && (keyPair.first != "Exposes");
+ em_utils::templateCharReplace(keyPair.second, dbusObject,
foundDeviceIdx, replaceStr,
handleLeftOver);
}
@@ -510,25 +563,30 @@
continue;
}
- for (auto& expose : *findExpose)
+ nlohmann::json::array_t* exposeArr =
+ findExpose->second.get_ptr<nlohmann::json::array_t*>();
+ if (exposeArr != nullptr)
{
- for (auto keyPair = expose.begin(); keyPair != expose.end();
- keyPair++)
+ for (auto& value : *exposeArr)
{
- em_utils::templateCharReplace(keyPair, dbusObject,
- foundDeviceIdx, replaceStr);
-
- applyExposeActions(_em.systemConfiguration, recordName, expose,
- keyPair);
+ applyTemplateAndExposeActions(recordName, dbusObject,
+ foundDeviceIdx, replaceStr, value,
+ _em.systemConfiguration);
}
}
+ else
+ {
+ applyTemplateAndExposeActions(
+ recordName, dbusObject, foundDeviceIdx, replaceStr,
+ findExpose->second, _em.systemConfiguration);
+ }
// If we end up here and the path is empty, we have Probe: "True"
// and we dont want that to show up in the associations.
if (!path.empty())
{
- auto boardType = record.find("Type")->get<std::string>();
- auto boardName = record.find("Name")->get<std::string>();
+ auto boardType = record.find("Type")->second.get<std::string>();
+ auto boardName = record.find("Name")->second.get<std::string>();
std::string boardInventoryPath =
em_utils::buildInventorySystemPath(boardName, boardType);
_em.topology.addProbePath(boardInventoryPath, path);
@@ -565,9 +623,16 @@
it = _configurations.erase(it);
continue;
}
- std::string probeName = *findName;
- if (std::find(passedProbes.begin(), passedProbes.end(), probeName) !=
+ const std::string* probeName = findName->get_ptr<const std::string*>();
+ if (probeName == nullptr)
+ {
+ lg2::error("Name wasn't a string? {JSON}", "JSON", *it);
+ it = _configurations.erase(it);
+ continue;
+ }
+
+ if (std::find(passedProbes.begin(), passedProbes.end(), *probeName) !=
passedProbes.end())
{
it = _configurations.erase(it);
@@ -575,40 +640,52 @@
}
nlohmann::json& recordRef = *it;
- nlohmann::json probeCommand;
- if ((*findProbe).type() != nlohmann::json::value_t::array)
+ std::vector<std::string> probeCommand;
+ nlohmann::json::array_t* probeCommandArrayPtr =
+ findProbe->get_ptr<nlohmann::json::array_t*>();
+ if (probeCommandArrayPtr != nullptr)
{
- probeCommand = nlohmann::json::array();
- probeCommand.push_back(*findProbe);
+ for (const auto& probe : *probeCommandArrayPtr)
+ {
+ const std::string* probeStr =
+ probe.get_ptr<const std::string*>();
+ if (probeStr == nullptr)
+ {
+ lg2::error("Probe statement wasn't a string, can't parse");
+ return;
+ }
+ probeCommand.push_back(*probeStr);
+ }
}
else
{
- probeCommand = *findProbe;
+ const std::string* probeStr =
+ findProbe->get_ptr<const std::string*>();
+ if (probeStr == nullptr)
+ {
+ lg2::error("Probe statement wasn't a string, can't parse");
+ return;
+ }
+ probeCommand.push_back(*probeStr);
}
// store reference to this to children to makes sure we don't get
// destroyed too early
auto thisRef = shared_from_this();
auto probePointer = std::make_shared<probe::PerformProbe>(
- recordRef, probeCommand, probeName, thisRef);
+ recordRef, probeCommand, *probeName, thisRef);
// parse out dbus probes by discarding other probe types, store in a
// map
- for (const nlohmann::json& probeJson : probeCommand)
+ for (const std::string& probe : probeCommand)
{
- const std::string* probe = probeJson.get_ptr<const std::string*>();
- if (probe == nullptr)
- {
- lg2::error("Probe statement wasn't a string, can't parse");
- continue;
- }
- if (probe::findProbeType(*probe))
+ if (probe::findProbeType(probe))
{
continue;
}
// syntax requires probe before first open brace
- auto findStart = probe->find('(');
- std::string interface = probe->substr(0, findStart);
+ auto findStart = probe.find('(');
+ std::string interface = probe.substr(0, findStart);
dbusProbeInterfaces.emplace(interface);
dbusProbePointers.emplace_back(probePointer);
}
@@ -630,7 +707,7 @@
std::move(_callback));
nextScan->passedProbes = std::move(passedProbes);
nextScan->dbusProbeObjects = std::move(dbusProbeObjects);
- nextScan->run();
+ boost::asio::post(_em.io, [nextScan]() { nextScan->run(); });
}
else
{
diff --git a/src/entity_manager/utils.cpp b/src/entity_manager/utils.cpp
index 2184fd7..a899f63 100644
--- a/src/entity_manager/utils.cpp
+++ b/src/entity_manager/utils.cpp
@@ -64,20 +64,30 @@
return false;
}
-void handleLeftOverTemplateVars(nlohmann::json::iterator& keyPair)
+void handleLeftOverTemplateVars(nlohmann::json& value)
{
- if (keyPair.value().type() == nlohmann::json::value_t::object ||
- keyPair.value().type() == nlohmann::json::value_t::array)
+ nlohmann::json::object_t* objPtr =
+ value.get_ptr<nlohmann::json::object_t*>();
+ if (objPtr != nullptr)
{
- for (auto nextLayer = keyPair.value().begin();
- nextLayer != keyPair.value().end(); nextLayer++)
+ for (auto& nextLayer : *objPtr)
+ {
+ handleLeftOverTemplateVars(nextLayer.second);
+ }
+ return;
+ }
+
+ nlohmann::json::array_t* arrPtr = value.get_ptr<nlohmann::json::array_t*>();
+ if (arrPtr != nullptr)
+ {
+ for (auto& nextLayer : *arrPtr)
{
handleLeftOverTemplateVars(nextLayer);
}
return;
}
- std::string* strPtr = keyPair.value().get_ptr<std::string*>();
+ std::string* strPtr = value.get_ptr<std::string*>();
if (strPtr == nullptr)
{
return;
@@ -125,25 +135,24 @@
// but checks all properties on all interfaces provided to do the substitution
// with.
std::optional<std::string> templateCharReplace(
- nlohmann::json::iterator& keyPair, const DBusObject& object,
- const size_t index, const std::optional<std::string>& replaceStr,
- bool handleLeftOver)
+ nlohmann::json& value, const DBusObject& object, const size_t index,
+ const std::optional<std::string>& replaceStr, bool handleLeftOver)
{
for (const auto& [_, interface] : object)
{
- auto ret = templateCharReplace(keyPair, interface, index, replaceStr);
+ auto ret = templateCharReplace(value, interface, index, replaceStr);
if (ret)
{
if (handleLeftOver)
{
- handleLeftOverTemplateVars(keyPair);
+ handleLeftOverTemplateVars(value);
}
return ret;
}
}
if (handleLeftOver)
{
- handleLeftOverTemplateVars(keyPair);
+ handleLeftOverTemplateVars(value);
}
return std::nullopt;
}
@@ -152,23 +161,33 @@
// the field found in a dbus object i.e. $ADDRESS would get populated with the
// ADDRESS field from a object on dbus
std::optional<std::string> templateCharReplace(
- nlohmann::json::iterator& keyPair, const DBusInterface& interface,
- const size_t index, const std::optional<std::string>& replaceStr)
+ nlohmann::json& value, const DBusInterface& interface, const size_t index,
+ const std::optional<std::string>& replaceStr)
{
std::optional<std::string> ret = std::nullopt;
- if (keyPair.value().type() == nlohmann::json::value_t::object ||
- keyPair.value().type() == nlohmann::json::value_t::array)
+ nlohmann::json::object_t* objPtr =
+ value.get_ptr<nlohmann::json::object_t*>();
+ if (objPtr != nullptr)
{
- for (auto nextLayer = keyPair.value().begin();
- nextLayer != keyPair.value().end(); nextLayer++)
+ for (auto& [key, value] : *objPtr)
{
- templateCharReplace(nextLayer, interface, index, replaceStr);
+ templateCharReplace(value, interface, index, replaceStr);
}
return ret;
}
- std::string* strPtr = keyPair.value().get_ptr<std::string*>();
+ nlohmann::json::array_t* arrPtr = value.get_ptr<nlohmann::json::array_t*>();
+ if (arrPtr != nullptr)
+ {
+ for (auto& value : *arrPtr)
+ {
+ templateCharReplace(value, interface, index, replaceStr);
+ }
+ return ret;
+ }
+
+ std::string* strPtr = value.get_ptr<std::string*>();
if (strPtr == nullptr)
{
return ret;
@@ -196,7 +215,7 @@
// check for additional operations
if ((start == 0U) && find.end() == strPtr->end())
{
- std::visit([&](auto&& val) { keyPair.value() = val; }, propValue);
+ std::visit([&](auto&& val) { value = val; }, propValue);
return ret;
}
@@ -255,18 +274,18 @@
{
result.append(" ").append(*exprEnd++);
}
- keyPair.value() = result;
+ value = result;
// We probably just invalidated the pointer abovei,
// reset and continue to handle multiple templates
- strPtr = keyPair.value().get_ptr<std::string*>();
+ strPtr = value.get_ptr<std::string*>();
if (strPtr == nullptr)
{
break;
}
}
- strPtr = keyPair.value().get_ptr<std::string*>();
+ strPtr = value.get_ptr<std::string*>();
if (strPtr == nullptr)
{
return ret;
@@ -286,7 +305,7 @@
fromCharsWrapper(strView, temp, fullMatch, base);
if (res.ec == std::errc{} && fullMatch)
{
- keyPair.value() = temp;
+ value = temp;
}
return ret;
diff --git a/src/entity_manager/utils.hpp b/src/entity_manager/utils.hpp
index a8bba16..14e35cc 100644
--- a/src/entity_manager/utils.hpp
+++ b/src/entity_manager/utils.hpp
@@ -27,16 +27,16 @@
bool fwVersionIsSame();
-void handleLeftOverTemplateVars(nlohmann::json::iterator& keyPair);
+void handleLeftOverTemplateVars(nlohmann::json& value);
std::optional<std::string> templateCharReplace(
- nlohmann::json::iterator& keyPair, const DBusObject& object, size_t index,
+ nlohmann::json& value, const DBusObject& object, size_t index,
const std::optional<std::string>& replaceStr = std::nullopt,
bool handleLeftOver = true);
std::optional<std::string> templateCharReplace(
- nlohmann::json::iterator& keyPair, const DBusInterface& interface,
- size_t index, const std::optional<std::string>& replaceStr = std::nullopt);
+ nlohmann::json& value, const DBusInterface& interface, size_t index,
+ const std::optional<std::string>& replaceStr = std::nullopt);
std::string buildInventorySystemPath(std::string& boardName,
const std::string& boardType);