Write the clang-tidy file OpenBMC needs
Now that CI can handle clang-tidy, and a lot of the individual fixes
have landed for the various static analysis checks, lets see how close
we are.
This includes bringing a bunch of the code up to par with the checks
that require. Most of them fall into the category of extraneous else
statements, const correctness problems, or extra copies.
Tested:
CI only. Unit tests pass.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9fbd346560a75fdd3901fa40c57932486275e912
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index d29d9c6..21b25e4 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -1064,7 +1064,7 @@
// send everything.
if (entry->resourceTypes.size())
{
- for (auto resource : entry->resourceTypes)
+ for (const auto& resource : entry->resourceTypes)
{
if (resType == resource)
{
diff --git a/redfish-core/include/privileges.hpp b/redfish-core/include/privileges.hpp
index 2d44545..c3a88d5 100644
--- a/redfish-core/include/privileges.hpp
+++ b/redfish-core/include/privileges.hpp
@@ -213,24 +213,21 @@
"ConfigureUsers", "ConfigureComponents"};
return admin;
}
- else if (userRole == "priv-operator")
+ if (userRole == "priv-operator")
{
// Redfish privilege : Operator
static Privileges op{"Login", "ConfigureSelf", "ConfigureComponents"};
return op;
}
- else if (userRole == "priv-user")
+ if (userRole == "priv-user")
{
// Redfish privilege : Readonly
static Privileges readOnly{"Login", "ConfigureSelf"};
return readOnly;
}
- else
- {
- // Redfish privilege : NoAccess
- static Privileges noaccess;
- return noaccess;
- }
+ // Redfish privilege : NoAccess
+ static Privileges noaccess;
+ return noaccess;
}
/**
diff --git a/redfish-core/include/utils/fw_utils.hpp b/redfish-core/include/utils/fw_utils.hpp
index 0750b43..5554d3b 100644
--- a/redfish-core/include/utils/fw_utils.hpp
+++ b/redfish-core/include/utils/fw_utils.hpp
@@ -284,21 +284,18 @@
{
return "Enabled";
}
- else if (fwState == "xyz.openbmc_project.Software.Activation."
- "Activations.Activating")
+ if (fwState == "xyz.openbmc_project.Software.Activation."
+ "Activations.Activating")
{
return "Updating";
}
- else if (fwState == "xyz.openbmc_project.Software.Activation."
- "Activations.StandbySpare")
+ if (fwState == "xyz.openbmc_project.Software.Activation."
+ "Activations.StandbySpare")
{
return "StandbySpare";
}
- else
- {
- BMCWEB_LOG_DEBUG << "Default fw state " << fwState << " to Disabled";
- return "Disabled";
- }
+ BMCWEB_LOG_DEBUG << "Default fw state " << fwState << " to Disabled";
+ return "Disabled";
}
/**
@@ -321,11 +318,8 @@
{
return "OK";
}
- else
- {
- BMCWEB_LOG_DEBUG << "FW state " << fwState << " to Warning";
- return "Warning";
- }
+ BMCWEB_LOG_DEBUG << "FW state " << fwState << " to Warning";
+ return "Warning";
}
/**
diff --git a/redfish-core/include/utils/systemd_utils.hpp b/redfish-core/include/utils/systemd_utils.hpp
index 0db1d94..75bbe35 100644
--- a/redfish-core/include/utils/systemd_utils.hpp
+++ b/redfish-core/include/utils/systemd_utils.hpp
@@ -29,7 +29,7 @@
* @return Service root UUID
*/
-inline const std::string getUuid()
+inline std::string getUuid()
{
std::string ret;
// This ID needs to match the one in ipmid
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 897c5c4..266843e 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -83,15 +83,15 @@
{
return "Administrator";
}
- else if (role == "priv-user")
+ if (role == "priv-user")
{
return "ReadOnly";
}
- else if (role == "priv-operator")
+ if (role == "priv-operator")
{
return "Operator";
}
- else if ((role == "") || (role == "priv-noaccess"))
+ if ((role == "") || (role == "priv-noaccess"))
{
return "NoAccess";
}
@@ -103,15 +103,15 @@
{
return "priv-admin";
}
- else if (role == "ReadOnly")
+ if (role == "ReadOnly")
{
return "priv-user";
}
- else if (role == "Operator")
+ if (role == "Operator")
{
return "priv-operator";
}
- else if ((role == "NoAccess") || (role == ""))
+ if ((role == "NoAccess") || (role == ""))
{
return "priv-noaccess";
}
@@ -185,7 +185,7 @@
{"GroupsAttribute", confData.groupAttribute}}}}},
};
- json_response[ldapType].update(std::move(ldap));
+ json_response[ldapType].update(ldap);
nlohmann::json& roleMapArray = json_response[ldapType]["RemoteRoleMapping"];
roleMapArray = nlohmann::json::array();
@@ -365,7 +365,7 @@
{"RemoteGroup", *remoteGroup}});
},
ldapDbusService, dbusObjectPath, ldapPrivMapperInterface,
- "Create", std::move(*remoteGroup),
+ "Create", *remoteGroup,
getPrivilegeFromRoleId(std::move(*localRole)));
}
}
@@ -1731,28 +1731,25 @@
locked);
return;
}
- else
- {
- crow::connections::systemBus->async_method_call(
- [this, asyncResp, username, password(std::move(password)),
- roleId(std::move(roleId)), enabled(std::move(enabled)),
- newUser{std::string(*newUserName)},
- locked(std::move(locked))](const boost::system::error_code ec,
- sdbusplus::message::message& m) {
- if (ec)
- {
- userErrorMessageHandler(m.get_error(), asyncResp,
- newUser, username);
- return;
- }
+ crow::connections::systemBus->async_method_call(
+ [this, asyncResp, username, password(std::move(password)),
+ roleId(std::move(roleId)), enabled(std::move(enabled)),
+ newUser{std::string(*newUserName)},
+ locked(std::move(locked))](const boost::system::error_code ec,
+ sdbusplus::message::message& m) {
+ if (ec)
+ {
+ userErrorMessageHandler(m.get_error(), asyncResp, newUser,
+ username);
+ return;
+ }
- updateUserProperties(asyncResp, newUser, password, enabled,
- roleId, locked);
- },
- "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
- "xyz.openbmc_project.User.Manager", "RenameUser", username,
- *newUserName);
- }
+ updateUserProperties(asyncResp, newUser, password, enabled,
+ roleId, locked);
+ },
+ "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
+ "xyz.openbmc_project.User.Manager", "RenameUser", username,
+ *newUserName);
}
void updateUserProperties(std::shared_ptr<AsyncResp> asyncResp,
diff --git a/redfish-core/lib/cpudimm.hpp b/redfish-core/lib/cpudimm.hpp
index e670037..e4b7a9d 100644
--- a/redfish-core/lib/cpudimm.hpp
+++ b/redfish-core/lib/cpudimm.hpp
@@ -103,11 +103,8 @@
// HTTP Code will be set up automatically, just return
return;
}
- else
- {
- aResp->res.jsonValue["Status"]["State"] = "Enabled";
- present = true;
- }
+ aResp->res.jsonValue["Status"]["State"] = "Enabled";
+ present = true;
aResp->res.jsonValue["TotalCores"] = *coresCount;
}
else if (property.first == "Socket")
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index 3494ede..1fb8783 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -158,11 +158,11 @@
{
return "xyz.openbmc_project.Network.EthernetInterface.DHCPConf.both";
}
- else if (isIPv4)
+ if (isIPv4)
{
return "xyz.openbmc_project.Network.EthernetInterface.DHCPConf.v4";
}
- else if (isIPv6)
+ if (isIPv6)
{
return "xyz.openbmc_project.Network.EthernetInterface.DHCPConf.v6";
}
@@ -183,10 +183,7 @@
{
return "IPv4LinkLocal";
}
- else
- {
- return "LinkLocal";
- }
+ return "LinkLocal";
}
if (inputOrigin == "xyz.openbmc_project.Network.IP.AddressOrigin.DHCP")
{
@@ -194,10 +191,7 @@
{
return "DHCP";
}
- else
- {
- return "DHCPv6";
- }
+ return "DHCPv6";
}
if (inputOrigin == "xyz.openbmc_project.Network.IP.AddressOrigin.SLAAC")
{
@@ -678,10 +672,7 @@
// Continuity not preserved
return false;
}
- else
- {
- (*bits)++;
- }
+ (*bits)++;
}
else
{
@@ -1567,12 +1558,9 @@
messages::resourceCannotBeDeleted(asyncResp->res);
return;
}
- else
- {
- messages::propertyValueFormatError(
- asyncResp->res, thisJson.dump(), pathString);
- return;
- }
+ messages::propertyValueFormatError(
+ asyncResp->res, thisJson.dump(), pathString);
+ return;
}
if (thisJson.is_null())
@@ -1704,12 +1692,9 @@
messages::resourceCannotBeDeleted(asyncResp->res);
return;
}
- else
- {
- messages::propertyValueFormatError(
- asyncResp->res, thisJson.dump(), pathString);
- return;
- }
+ messages::propertyValueFormatError(
+ asyncResp->res, thisJson.dump(), pathString);
+ return;
}
if (thisJson.is_null())
@@ -2099,10 +2084,7 @@
{
return false;
}
- else
- {
- return true;
- }
+ return true;
}
/**
diff --git a/redfish-core/lib/health.hpp b/redfish-core/lib/health.hpp
index 4f21b3e..19175e7 100644
--- a/redfish-core/lib/health.hpp
+++ b/redfish-core/lib/health.hpp
@@ -122,8 +122,8 @@
rollup = "Critical";
return;
}
- else if (boost::starts_with(path.str, globalInventoryPath) &&
- boost::ends_with(path.str, "warning"))
+ if (boost::starts_with(path.str, globalInventoryPath) &&
+ boost::ends_with(path.str, "warning"))
{
health = "Warning";
if (rollup != "Critical")
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index 777f37e..1198455 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -114,13 +114,13 @@
{
return "Critical";
}
- else if ((s == "xyz.openbmc_project.Logging.Entry.Level.Debug") ||
- (s == "xyz.openbmc_project.Logging.Entry.Level.Informational") ||
- (s == "xyz.openbmc_project.Logging.Entry.Level.Notice"))
+ if ((s == "xyz.openbmc_project.Logging.Entry.Level.Debug") ||
+ (s == "xyz.openbmc_project.Logging.Entry.Level.Informational") ||
+ (s == "xyz.openbmc_project.Logging.Entry.Level.Notice"))
{
return "OK";
}
- else if (s == "xyz.openbmc_project.Logging.Entry.Level.Warning")
+ if (s == "xyz.openbmc_project.Logging.Entry.Level.Warning")
{
return "Warning";
}
@@ -789,8 +789,8 @@
"DiagnosticDataType & OEMDiagnosticDataType");
return;
}
- else if ((*oemDiagnosticDataType != "System") ||
- (*diagnosticDataType != "OEM"))
+ if ((*oemDiagnosticDataType != "System") ||
+ (*diagnosticDataType != "OEM"))
{
BMCWEB_LOG_ERROR << "Wrong parameter values passed";
messages::invalidObject(asyncResp->res,
@@ -808,7 +808,7 @@
asyncResp->res, "CollectDiagnosticData", "DiagnosticDataType");
return;
}
- else if (*diagnosticDataType != "Manager")
+ if (*diagnosticDataType != "Manager")
{
BMCWEB_LOG_ERROR
<< "Wrong parameter value passed for 'DiagnosticDataType'";
diff --git a/redfish-core/lib/managers.hpp b/redfish-core/lib/managers.hpp
index 213237c..2da7f3a 100644
--- a/redfish-core/lib/managers.hpp
+++ b/redfish-core/lib/managers.hpp
@@ -133,21 +133,18 @@
doBMCGracefulRestart(asyncResp);
return;
}
- else if (resetType == "ForceRestart")
+ if (resetType == "ForceRestart")
{
BMCWEB_LOG_DEBUG << "Proceeding with " << resetType;
doBMCForceRestart(asyncResp);
return;
}
- else
- {
- BMCWEB_LOG_DEBUG << "Invalid property value for ResetType: "
- << resetType;
- messages::actionParameterNotSupported(asyncResp->res, resetType,
- "ResetType");
+ BMCWEB_LOG_DEBUG << "Invalid property value for ResetType: "
+ << resetType;
+ messages::actionParameterNotSupported(asyncResp->res, resetType,
+ "ResetType");
- return;
- }
+ return;
}
};
@@ -1611,7 +1608,7 @@
{
return;
}
- else if (ret == CreatePIDRet::del)
+ if (ret == CreatePIDRet::del)
{
continue;
}
diff --git a/redfish-core/lib/network_protocol.hpp b/redfish-core/lib/network_protocol.hpp
index 8ee617f..8752cf8 100644
--- a/redfish-core/lib/network_protocol.hpp
+++ b/redfish-core/lib/network_protocol.hpp
@@ -265,7 +265,8 @@
for (auto& unit : r)
{
/* Only traverse through <xyz>.socket units */
- std::string unitName = std::get<NET_PROTO_UNIT_NAME>(unit);
+ const std::string& unitName =
+ std::get<NET_PROTO_UNIT_NAME>(unit);
if (!boost::ends_with(unitName, ".socket"))
{
continue;
@@ -280,9 +281,9 @@
continue;
}
const char* rfServiceKey = kv.first;
- std::string socketPath =
+ const std::string& socketPath =
std::get<NET_PROTO_UNIT_OBJ_PATH>(unit);
- std::string unitState =
+ const std::string& unitState =
std::get<NET_PROTO_UNIT_SUB_STATE>(unit);
asyncResp->res
diff --git a/redfish-core/lib/pcie.hpp b/redfish-core/lib/pcie.hpp
index 21ec6dc..19438af 100644
--- a/redfish-core/lib/pcie.hpp
+++ b/redfish-core/lib/pcie.hpp
@@ -45,7 +45,7 @@
pcieDeviceList = nlohmann::json::array();
for (const std::string& pcieDevicePath : pcieDevicePaths)
{
- size_t devStart = pcieDevicePath.rfind("/");
+ size_t devStart = pcieDevicePath.rfind('/');
if (devStart == std::string::npos)
{
continue;
diff --git a/redfish-core/lib/roles.hpp b/redfish-core/lib/roles.hpp
index 610c086..de2e1ff 100644
--- a/redfish-core/lib/roles.hpp
+++ b/redfish-core/lib/roles.hpp
@@ -28,15 +28,15 @@
{
return "Administrator";
}
- else if (priv == "priv-user")
+ if (priv == "priv-user")
{
return "ReadOnly";
}
- else if (priv == "priv-operator")
+ if (priv == "priv-operator")
{
return "Operator";
}
- else if (priv == "priv-noaccess")
+ if (priv == "priv-noaccess")
{
return "NoAccess";
}
diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp
index ac98735..91e54bb 100644
--- a/redfish-core/lib/sensors.hpp
+++ b/redfish-core/lib/sensors.hpp
@@ -304,9 +304,7 @@
auto objectsWithConnectionCb =
[callback](const boost::container::flat_set<std::string>& connections,
const std::set<std::pair<std::string, std::string>>&
- /*objectsWithConnection*/) {
- callback(std::move(connections));
- };
+ /*objectsWithConnection*/) { callback(connections); };
getObjectsWithConnection(SensorsAsyncResp, sensorNames,
std::move(objectsWithConnectionCb));
}
diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp
index 34202b1..3f74ca5 100644
--- a/redfish-core/lib/systems.hpp
+++ b/redfish-core/lib/systems.hpp
@@ -351,7 +351,9 @@
std::get_if<uint64_t>(
&property.second);
if (nullptr != procFamily)
+ {
break;
+ }
continue;
}
@@ -361,7 +363,9 @@
std::get_if<std::string>(
&property.second);
if (nullptr != processorId)
+ {
break;
+ }
continue;
}
}
@@ -670,30 +674,25 @@
{
return "None";
}
- else if (dbusSource ==
- "xyz.openbmc_project.Control.Boot.Source.Sources.Disk")
+ if (dbusSource == "xyz.openbmc_project.Control.Boot.Source.Sources.Disk")
{
return "Hdd";
}
- else if (dbusSource ==
- "xyz.openbmc_project.Control.Boot.Source.Sources.ExternalMedia")
+ if (dbusSource ==
+ "xyz.openbmc_project.Control.Boot.Source.Sources.ExternalMedia")
{
return "Cd";
}
- else if (dbusSource ==
- "xyz.openbmc_project.Control.Boot.Source.Sources.Network")
+ if (dbusSource == "xyz.openbmc_project.Control.Boot.Source.Sources.Network")
{
return "Pxe";
}
- else if (dbusSource ==
- "xyz.openbmc_project.Control.Boot.Source.Sources.RemovableMedia")
+ if (dbusSource ==
+ "xyz.openbmc_project.Control.Boot.Source.Sources.RemovableMedia")
{
return "Usb";
}
- else
- {
- return "";
- }
+ return "";
}
/**
@@ -710,18 +709,15 @@
{
return "None";
}
- else if (dbusMode == "xyz.openbmc_project.Control.Boot.Mode.Modes.Safe")
+ if (dbusMode == "xyz.openbmc_project.Control.Boot.Mode.Modes.Safe")
{
return "Diags";
}
- else if (dbusMode == "xyz.openbmc_project.Control.Boot.Mode.Modes.Setup")
+ if (dbusMode == "xyz.openbmc_project.Control.Boot.Mode.Modes.Setup")
{
return "BiosSetup";
}
- else
- {
- return "";
- }
+ return "";
}
/**
@@ -747,7 +743,7 @@
{
return 0;
}
- else if (rfSource == "Pxe")
+ if (rfSource == "Pxe")
{
bootSource = "xyz.openbmc_project.Control.Boot.Source.Sources.Network";
}
@@ -1482,17 +1478,15 @@
{
return "None";
}
- else if (dbusAction ==
- "xyz.openbmc_project.State.Watchdog.Action.HardReset")
+ if (dbusAction == "xyz.openbmc_project.State.Watchdog.Action.HardReset")
{
return "ResetSystem";
}
- else if (dbusAction == "xyz.openbmc_project.State.Watchdog.Action.PowerOff")
+ if (dbusAction == "xyz.openbmc_project.State.Watchdog.Action.PowerOff")
{
return "PowerDown";
}
- else if (dbusAction ==
- "xyz.openbmc_project.State.Watchdog.Action.PowerCycle")
+ if (dbusAction == "xyz.openbmc_project.State.Watchdog.Action.PowerCycle")
{
return "PowerCycle";
}
@@ -1515,15 +1509,15 @@
{
return "xyz.openbmc_project.State.Watchdog.Action.None";
}
- else if (rfAction == "PowerCycle")
+ if (rfAction == "PowerCycle")
{
return "xyz.openbmc_project.State.Watchdog.Action.PowerCycle";
}
- else if (rfAction == "PowerDown")
+ if (rfAction == "PowerDown")
{
return "xyz.openbmc_project.State.Watchdog.Action.PowerOff";
}
- else if (rfAction == "ResetSystem")
+ if (rfAction == "ResetSystem")
{
return "xyz.openbmc_project.State.Watchdog.Action.HardReset";
}
diff --git a/redfish-core/ut/lock_test.cpp b/redfish-core/ut/lock_test.cpp
index 08ba27a..5c68cdc 100644
--- a/redfish-core/ut/lock_test.cpp
+++ b/redfish-core/ut/lock_test.cpp
@@ -59,8 +59,8 @@
234,
{{"LockAll", 2}, {"DontLock", 4}}}};
}
- ~LockTest()
- {}
+
+ ~LockTest() override = default;
};
class MockLock : public crow::ibm_mc_lock::Lock