From f8fe53e7114ab10c9059377541277739ace5c1ff Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Thu, 30 Jun 2022 15:55:45 -0700 Subject: Change variable scopes cppcheck correctly notes that a lot of our variables can be declared at more specific scopes, and in every case, it seems to be correct. Tested: Redfish service validator passes. Unit test coverage on others. Signed-off-by: Ed Tanous Change-Id: Ia4414410d0e8f74a3bd40fdc0e0232450d1a6416 --- http/http_response.hpp | 4 ++-- http/utility.hpp | 3 +-- include/dbus_monitor.hpp | 8 +++----- include/http_utility.hpp | 3 +-- include/ibm/locks.hpp | 5 +---- include/openbmc_dbus_rest.hpp | 3 +-- redfish-core/include/utils/chassis_utils.hpp | 3 +-- redfish-core/lib/health.hpp | 2 +- redfish-core/lib/hypervisor_system.hpp | 5 ++--- redfish-core/lib/power.hpp | 10 ++++++---- redfish-core/lib/processor.hpp | 2 +- redfish-core/lib/sensors.hpp | 3 +-- 12 files changed, 21 insertions(+), 30 deletions(-) diff --git a/http/http_response.hpp b/http/http_response.hpp index c1f25a543c..b1b15fb362 100644 --- a/http/http_response.hpp +++ b/http/http_response.hpp @@ -41,9 +41,9 @@ struct Response {} Response(Response&& res) noexcept : - stringResponse(std::move(res.stringResponse)), completed(res.completed) + stringResponse(std::move(res.stringResponse)), + jsonValue(std::move(res.jsonValue)), completed(res.completed) { - jsonValue = std::move(res.jsonValue); // See note in operator= move handler for why this is needed. if (!res.completed) { diff --git a/http/utility.hpp b/http/utility.hpp index 4f5cea74fd..f49aa9b859 100644 --- a/http/utility.hpp +++ b/http/utility.hpp @@ -485,7 +485,6 @@ inline bool base64Decode(const std::string_view input, std::string& output) char base64code0 = 0; char base64code1 = 0; char base64code2 = 0; // initialized to 0 to suppress warnings - char base64code3 = 0; base64code0 = getCodeValue(input[i]); if (base64code0 == nop) @@ -528,7 +527,7 @@ inline bool base64Decode(const std::string_view input, std::string& output) { // padding , end of input return (base64code2 & 0x03) == 0; } - base64code3 = getCodeValue(input[i]); + char base64code3 = getCodeValue(input[i]); if (base64code3 == nop) { // non base64 character return false; diff --git a/include/dbus_monitor.hpp b/include/dbus_monitor.hpp index 5f409d7d70..0838f0ae6c 100644 --- a/include/dbus_monitor.hpp +++ b/include/dbus_monitor.hpp @@ -167,9 +167,7 @@ inline void requestRoutes(App& app) // PropertiesChanged thisSession.matches.reserve(thisSession.matches.size() + paths->size() * (1U + interfaceCount)); - std::string objectManagerMatchString; - std::string propertiesMatchString; - std::string objectManagerInterfacesMatchString; + // These regexes derived on the rules here: // https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-names std::regex validPath("^/([A-Za-z0-9_]+/?)*$"); @@ -192,7 +190,7 @@ inline void requestRoutes(App& app) conn.close(); return; } - propertiesMatchString = + std::string propertiesMatchString = ("type='signal'," "interface='org.freedesktop.DBus.Properties'," "path_namespace='" + @@ -234,7 +232,7 @@ inline void requestRoutes(App& app) onPropertyUpdate, &conn)); } } - objectManagerMatchString = + std::string objectManagerMatchString = ("type='signal'," "interface='org.freedesktop.DBus.ObjectManager'," "path_namespace='" + diff --git a/include/http_utility.hpp b/include/http_utility.hpp index cd1be3e2b1..e1a8e2a9ea 100644 --- a/include/http_utility.hpp +++ b/include/http_utility.hpp @@ -44,11 +44,10 @@ constexpr std::array contentTypes{{ inline ContentType getPreferedContentType(std::string_view header, std::span preferedOrder) { - size_t index = 0; size_t lastIndex = 0; while (lastIndex < header.size() + 1) { - index = header.find(',', lastIndex); + size_t index = header.find(',', lastIndex); if (index == std::string_view::npos) { index = header.size(); diff --git a/include/ibm/locks.hpp b/include/ibm/locks.hpp index e713f97b8e..e3abb46e92 100644 --- a/include/ibm/locks.hpp +++ b/include/ibm/locks.hpp @@ -431,12 +431,9 @@ inline bool Lock::isValidLockRequest(const LockRequest& refLockRecord) inline Rc Lock::isConflictWithTable(const LockRequests& refLockRequestStructure) { - - uint32_t thisTransactionId = 0; - if (lockTable.empty()) { - thisTransactionId = generateTransactionId(); + uint32_t thisTransactionId = generateTransactionId(); BMCWEB_LOG_DEBUG << thisTransactionId; // Lock table is empty, so we are safe to add the lockrecords // as there will be no conflict diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp index 95503d706e..0e3e070ccd 100644 --- a/include/openbmc_dbus_rest.hpp +++ b/include/openbmc_dbus_rest.hpp @@ -432,7 +432,6 @@ inline void getObjectAndEnumerate( { for (const auto& connection : object.second) { - std::string& objectManagerPath = connections[connection.first]; for (const auto& interface : connection.second) { BMCWEB_LOG_DEBUG << connection.first << " has interface " @@ -441,7 +440,7 @@ inline void getObjectAndEnumerate( { BMCWEB_LOG_DEBUG << "found object manager path " << object.first; - objectManagerPath = object.first; + connections[connection.first] = object.first; } } } diff --git a/redfish-core/include/utils/chassis_utils.hpp b/redfish-core/include/utils/chassis_utils.hpp index 0acbf33b7a..b7f16d83d0 100644 --- a/redfish-core/include/utils/chassis_utils.hpp +++ b/redfish-core/include/utils/chassis_utils.hpp @@ -35,11 +35,10 @@ void getValidChassisPath(const std::shared_ptr& asyncResp, } std::optional chassisPath; - std::string chassisName; for (const std::string& chassis : chassisPaths) { sdbusplus::message::object_path path(chassis); - chassisName = path.filename(); + std::string chassisName = path.filename(); if (chassisName.empty()) { BMCWEB_LOG_ERROR << "Failed to find '/' in " << chassis; diff --git a/redfish-core/lib/health.hpp b/redfish-core/lib/health.hpp index 2f0f156ae0..7955fc0d4b 100644 --- a/redfish-core/lib/health.hpp +++ b/redfish-core/lib/health.hpp @@ -67,7 +67,6 @@ struct HealthPopulate : std::enable_shared_from_this for (const auto& [path, interfaces] : statuses) { - bool isChild = false; bool isSelf = false; if (selfPath) { @@ -86,6 +85,7 @@ struct HealthPopulate : std::enable_shared_from_this // of this association is an inventory item, or one of the // endpoints in this association is a child + bool isChild = false; for (const std::string& child : inventory) { if (path.str.starts_with(child)) diff --git a/redfish-core/lib/hypervisor_system.hpp b/redfish-core/lib/hypervisor_system.hpp index 78bf6269a5..2b27fb4824 100644 --- a/redfish-core/lib/hypervisor_system.hpp +++ b/redfish-core/lib/hypervisor_system.hpp @@ -585,11 +585,10 @@ inline void handleHypervisorIPv4StaticPatch( // address const nlohmann::json& thisJson = input[0]; - // For the error string - std::string pathString = "IPv4StaticAddresses/1"; - if (!thisJson.is_null() && !thisJson.empty()) { + // For the error string + std::string pathString = "IPv4StaticAddresses/1"; std::optional address; std::optional subnetMask; std::optional gateway; diff --git a/redfish-core/lib/power.hpp b/redfish-core/lib/power.hpp index 47f0c5d24f..33aefa3978 100644 --- a/redfish-core/lib/power.hpp +++ b/redfish-core/lib/power.hpp @@ -273,9 +273,6 @@ inline void requestRoutesPower(App& app) } } - nlohmann::json& value = - sensorJson["PowerLimit"]["LimitInWatts"]; - // LimitException is Mandatory attribute as per OCP // Baseline Profile – v1.0.0, so currently making it // "NoAction" as default value to make it OCP Compliant. @@ -285,7 +282,12 @@ inline void requestRoutesPower(App& app) { // Redfish specification indicates PowerLimit should // be null if the limit is not enabled. - value = powerCap * std::pow(10, scale); + sensorJson["PowerLimit"]["LimitInWatts"] = + powerCap * std::pow(10, scale); + } + else + { + sensorJson["PowerLimit"]["LimitInWatts"] = nullptr; } }; diff --git a/redfish-core/lib/processor.hpp b/redfish-core/lib/processor.hpp index b6c500e814..d9c26f3d86 100644 --- a/redfish-core/lib/processor.hpp +++ b/redfish-core/lib/processor.hpp @@ -1294,9 +1294,9 @@ inline void requestRoutesProcessor(App& app) return; } - std::string appliedConfigUri; if (appliedConfigJson) { + std::string appliedConfigUri; if (!json_util::readJson(*appliedConfigJson, asyncResp->res, "@odata.id", appliedConfigUri)) { diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp index 3cd437982d..627ba7e50e 100644 --- a/redfish-core/lib/sensors.hpp +++ b/redfish-core/lib/sensors.hpp @@ -531,11 +531,10 @@ void getChassis(const std::shared_ptr& asyncResp, return; } const std::string* chassisPath = nullptr; - std::string chassisName; for (const std::string& chassis : chassisPaths) { sdbusplus::message::object_path path(chassis); - chassisName = path.filename(); + std::string chassisName = path.filename(); if (chassisName.empty()) { BMCWEB_LOG_ERROR << "Failed to find '/' in " << chassis; -- cgit v1.2.3