Add readability-redundant-* checks
There's a number of redundancies in our code that clang can sanitize
out. Fix the existing problems, and enable the checks.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie63d7b7f0777b702fbf1b23a24e1bed7b4f5183b
diff --git a/.clang-tidy b/.clang-tidy
index 953c78e..23cc7ae 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -276,6 +276,13 @@
readability-else-after-return,
readability-named-parameter,
readability-redundant-control-flow,
+readability-redundant-declaration,
+readability-redundant-function-ptr-dereference,
+readability-redundant-member-init,
+readability-redundant-preprocessor,
+readability-redundant-smartptr-get,
+readability-redundant-string-cstr,
+readability-redundant-string-init,
readability-identifier-naming'
WarningsAsErrors: '*'
diff --git a/http/app.hpp b/http/app.hpp
index 03de3f8..c1472e5 100644
--- a/http/app.hpp
+++ b/http/app.hpp
@@ -142,7 +142,7 @@
std::vector<const std::string*> getRoutes()
{
- const std::string root("");
+ const std::string root;
return router.getRoutes(root);
}
std::vector<const std::string*> getRoutes(const std::string& parent)
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 0b58e9b..61d3f7b 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -261,7 +261,7 @@
return true;
}
sslUser.resize(lastChar);
- std::string unsupportedClientId = "";
+ std::string unsupportedClientId;
sessionIsFromTransport = true;
userSession = persistent_data::SessionStore::getInstance()
.generateUserSession(
@@ -725,7 +725,6 @@
BMCWEB_LOG_DEBUG << this << " timer started";
}
- private:
Adaptor adaptor;
Handler* handler;
// Making this a std::optional allows it to be efficiently destroyed and
diff --git a/http/websocket.hpp b/http/websocket.hpp
index eadb276..213c4c7 100644
--- a/http/websocket.hpp
+++ b/http/websocket.hpp
@@ -77,7 +77,7 @@
std::function<void(Connection&, const std::string&)> closeHandler,
std::function<void(Connection&)> errorHandler) :
Connection(reqIn, reqIn.session->username),
- ws(std::move(adaptorIn)), inString(), inBuffer(inString, 131088),
+ ws(std::move(adaptorIn)), inBuffer(inString, 131088),
openHandler(std::move(openHandler)),
messageHandler(std::move(messageHandler)),
closeHandler(std::move(closeHandler)),
diff --git a/include/authorization.hpp b/include/authorization.hpp
index 27d7b32..f8e74b9 100644
--- a/include/authorization.hpp
+++ b/include/authorization.hpp
@@ -85,7 +85,7 @@
// needed.
// This whole flow needs to be revisited anyway, as we can't be
// calling directly into pam for every request
- std::string unsupportedClientId = "";
+ std::string unsupportedClientId;
return persistent_data::SessionStore::getInstance().generateUserSession(
user, clientIp, unsupportedClientId,
persistent_data::PersistenceType::SINGLE_REQUEST, isConfigureSelfOnly);
diff --git a/include/ibm/management_console_rest.hpp b/include/ibm/management_console_rest.hpp
index e80e727..39146c9 100644
--- a/include/ibm/management_console_rest.hpp
+++ b/include/ibm/management_console_rest.hpp
@@ -512,7 +512,8 @@
asyncResp->res.result(boost::beast::http::status::conflict);
auto var =
std::get<std::pair<uint32_t, LockRequest>>(conflictStatus.second);
- nlohmann::json returnJson, segments;
+ nlohmann::json returnJson;
+ nlohmann::json segments;
nlohmann::json myarray = nlohmann::json::array();
returnJson["TransactionID"] = var.first;
returnJson["SessionID"] = std::get<0>(var.second);
@@ -579,7 +580,8 @@
asyncResp->res.result(boost::beast::http::status::unauthorized);
auto var = statusRelease.second;
- nlohmann::json returnJson, segments;
+ nlohmann::json returnJson;
+ nlohmann::json segments;
nlohmann::json myArray = nlohmann::json::array();
returnJson["TransactionID"] = var.first;
returnJson["SessionID"] = std::get<0>(var.second);
diff --git a/include/login_routes.hpp b/include/login_routes.hpp
index c918cdd..1087b0b 100644
--- a/include/login_routes.hpp
+++ b/include/login_routes.hpp
@@ -185,7 +185,7 @@
}
else
{
- std::string unsupportedClientId = "";
+ std::string unsupportedClientId;
auto session =
persistent_data::SessionStore::getInstance()
.generateUserSession(
diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp
index 8aab757..6861571 100644
--- a/include/openbmc_dbus_rest.hpp
+++ b/include/openbmc_dbus_rest.hpp
@@ -2044,7 +2044,7 @@
// If accessing a single attribute, fill in and update objectPath,
// otherwise leave destProperty blank
- std::string destProperty = "";
+ std::string destProperty;
const char* attrSeperator = "/attr/";
size_t attrPosition = objectPath.find(attrSeperator);
if (attrPosition != objectPath.npos)
diff --git a/include/persistent_data.hpp b/include/persistent_data.hpp
index 47bb8a9..02f2cce 100644
--- a/include/persistent_data.hpp
+++ b/include/persistent_data.hpp
@@ -289,7 +289,7 @@
persistentFile << data;
}
- std::string systemUuid{""};
+ std::string systemUuid;
};
inline ConfigFile& getConfig()
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index ec294ae..3429c02 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -1356,7 +1356,7 @@
for (const auto& it :
EventServiceManager::getInstance().subscriptionsMap)
{
- Subscription& entry = *it.second.get();
+ Subscription& entry = *it.second;
if (entry.eventFormatType == metricReportFormatType)
{
entry.filterAndSendReports(id, *readings);
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 8770782..5530e12 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -1191,7 +1191,7 @@
messages::success(asyncResp->res);
return;
},
- "xyz.openbmc_project.User.Manager", dbusObjectPath.c_str(),
+ "xyz.openbmc_project.User.Manager", dbusObjectPath,
"org.freedesktop.DBus.Properties", "Set",
"xyz.openbmc_project.User.Attributes", "UserEnabled",
dbus::utility::DbusVariantType{*enabled});
@@ -1221,7 +1221,7 @@
}
messages::success(asyncResp->res);
},
- "xyz.openbmc_project.User.Manager", dbusObjectPath.c_str(),
+ "xyz.openbmc_project.User.Manager", dbusObjectPath,
"org.freedesktop.DBus.Properties", "Set",
"xyz.openbmc_project.User.Attributes", "UserPrivilege",
dbus::utility::DbusVariantType{priv});
@@ -1250,7 +1250,7 @@
messages::success(asyncResp->res);
return;
},
- "xyz.openbmc_project.User.Manager", dbusObjectPath.c_str(),
+ "xyz.openbmc_project.User.Manager", dbusObjectPath,
"org.freedesktop.DBus.Properties", "Set",
"xyz.openbmc_project.User.Attributes",
"UserLockedForFailedAttempt",
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index f4260a8..5283fff 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -2188,7 +2188,7 @@
}
};
- if (vlanEnable == true)
+ if (vlanEnable)
{
crow::connections::systemBus->async_method_call(
std::move(callback),
diff --git a/redfish-core/lib/hypervisor_system.hpp b/redfish-core/lib/hypervisor_system.hpp
index 588cf13..645e39f 100644
--- a/redfish-core/lib/hypervisor_system.hpp
+++ b/redfish-core/lib/hypervisor_system.hpp
@@ -541,7 +541,7 @@
// Set the IPv4 address origin to the DHCP / Static as per the new value
// of the DHCPEnabled property
std::string origin;
- if (ipv4DHCPEnabled == false)
+ if (!ipv4DHCPEnabled)
{
origin = "xyz.openbmc_project.Network.IP.AddressOrigin.Static";
}
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index aa4d5ec..84e6b34 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -676,7 +676,7 @@
entryID + "/attachment";
}
}
- if (foundDumpEntry == false)
+ if (!foundDumpEntry)
{
BMCWEB_LOG_ERROR << "Can't find Dump Entry";
messages::internalError(asyncResp->res);
diff --git a/redfish-core/lib/memory.hpp b/redfish-core/lib/memory.hpp
index 49304e2..f20681b 100644
--- a/redfish-core/lib/memory.hpp
+++ b/redfish-core/lib/memory.hpp
@@ -546,7 +546,7 @@
<< "Invalid property type for Dimm Presence";
return;
}
- if (*value == false)
+ if (!*value)
{
aResp->res.jsonValue["Status"]["State"] = "Absent";
}
diff --git a/redfish-core/lib/processor.hpp b/redfish-core/lib/processor.hpp
index 8e07881..b51a901 100644
--- a/redfish-core/lib/processor.hpp
+++ b/redfish-core/lib/processor.hpp
@@ -92,7 +92,7 @@
messages::internalError(aResp->res);
return;
}
- if (*cpuPresent == false)
+ if (!*cpuPresent)
{
// Slot is not populated
aResp->res.jsonValue["Status"]["State"] = "Absent";
@@ -106,7 +106,7 @@
messages::internalError(aResp->res);
return;
}
- if (*cpuFunctional == false)
+ if (!*cpuFunctional)
{
aResp->res.jsonValue["Status"]["Health"] = "Critical";
}
@@ -248,7 +248,7 @@
std::get_if<bool>(&property.second);
if (present != nullptr)
{
- if (*present == true)
+ if (*present)
{
slotPresent = true;
totalCores++;
@@ -442,12 +442,12 @@
std::string state = "Enabled";
std::string health = "OK";
- if (accPresent != nullptr && *accPresent == false)
+ if (accPresent != nullptr && !*accPresent)
{
state = "Absent";
}
- if ((accFunctional != nullptr) && (*accFunctional == false))
+ if ((accFunctional != nullptr) && !*accFunctional)
{
if (state == "Enabled")
{
diff --git a/redfish-core/lib/roles.hpp b/redfish-core/lib/roles.hpp
index c18942f..6c67a33 100644
--- a/redfish-core/lib/roles.hpp
+++ b/redfish-core/lib/roles.hpp
@@ -81,7 +81,7 @@
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::string& roleId) {
nlohmann::json privArray = nlohmann::json::array();
- if (false == getAssignedPrivFromRole(roleId, privArray))
+ if (!getAssignedPrivFromRole(roleId, privArray))
{
messages::resourceNotFound(asyncResp->res, "Role", roleId);
diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp
index f3fa52c..4d4b4cc 100644
--- a/redfish-core/lib/sensors.hpp
+++ b/redfish-core/lib/sensors.hpp
@@ -284,11 +284,7 @@
class InventoryItem
{
public:
- InventoryItem(const std::string& objPath) :
- objectPath(objPath), name(), isPresent(true), isFunctional(true),
- isPowerSupply(false), powerSupplyEfficiencyPercent(-1), manufacturer(),
- model(), partNumber(), serialNumber(), sensors(), ledObjectPath(""),
- ledState(LedState::UNKNOWN)
+ InventoryItem(const std::string& objPath) : objectPath(objPath)
{
// Set inventory item name to last node of object path
sdbusplus::message::object_path path(objectPath);
@@ -301,17 +297,17 @@
std::string objectPath;
std::string name;
- bool isPresent;
- bool isFunctional;
- bool isPowerSupply;
- int powerSupplyEfficiencyPercent;
+ bool isPresent = true;
+ bool isFunctional = true;
+ bool isPowerSupply = false;
+ int powerSupplyEfficiencyPercent = -1;
std::string manufacturer;
std::string model;
std::string partNumber;
std::string serialNumber;
std::set<std::string> sensors;
std::string ledObjectPath;
- LedState ledState;
+ LedState ledState = LedState::UNKNOWN;
};
/**
diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp
index 4cc3d8b..d680537 100644
--- a/redfish-core/lib/systems.hpp
+++ b/redfish-core/lib/systems.hpp
@@ -55,7 +55,7 @@
aResp->res.jsonValue["MemorySummary"]["Status"]["State"];
if (prevMemSummary == "Disabled")
{
- if (isDimmFunctional == true)
+ if (isDimmFunctional)
{
aResp->res.jsonValue["MemorySummary"]["Status"]["State"] =
"Enabled";
@@ -114,7 +114,7 @@
// Functional.
if (prevProcState == "Disabled")
{
- if (isCpuFunctional == true)
+ if (isCpuFunctional)
{
aResp->res.jsonValue["ProcessorSummary"]["Status"]["State"] =
"Enabled";
@@ -1129,7 +1129,7 @@
}
BMCWEB_LOG_DEBUG << "Auto Reboot: " << autoRebootEnabled;
- if (autoRebootEnabled == true)
+ if (autoRebootEnabled)
{
aResp->res.jsonValue["Boot"]["AutomaticRetryConfig"] =
"RetryAttempts";
@@ -2490,7 +2490,7 @@
return;
}
- if (parseIpsProperties(aResp, properties) == false)
+ if (!parseIpsProperties(aResp, properties))
{
messages::internalError(aResp->res);
return;
diff --git a/redfish-core/lib/update_service.hpp b/redfish-core/lib/update_service.hpp
index ba92748..4b18520 100644
--- a/redfish-core/lib/update_service.hpp
+++ b/redfish-core/lib/update_service.hpp
@@ -263,7 +263,7 @@
int timeoutTimeSeconds = 10)
{
// Only allow one FW update at a time
- if (fwUpdateInProgress != false)
+ if (fwUpdateInProgress)
{
if (asyncResp)
{
@@ -819,7 +819,7 @@
std::string, std::vector<std::string>>>>& obj :
subtree)
{
- if (boost::ends_with(obj.first, *swId) != true)
+ if (!boost::ends_with(obj.first, *swId))
{
continue;
}
diff --git a/redfish-core/lib/virtual_media.hpp b/redfish-core/lib/virtual_media.hpp
index d646698..ab27fc7 100644
--- a/redfish-core/lib/virtual_media.hpp
+++ b/redfish-core/lib/virtual_media.hpp
@@ -133,7 +133,7 @@
}
aResp->res.jsonValue["Inserted"] = *activeValue;
- if (*activeValue == true)
+ if (*activeValue)
{
aResp->res.jsonValue["ConnectedVia"] = "Applet";
}
@@ -400,7 +400,7 @@
}
// optional param inserted must be true
- if ((inserted != std::nullopt) && (*inserted != true))
+ if ((inserted != std::nullopt) && !*inserted)
{
BMCWEB_LOG_ERROR
<< "Request action optional parameter Inserted must be true.";
@@ -814,7 +814,7 @@
actionParams.inserted, actionParams.transferMethod,
actionParams.transferProtocolType);
- if (paramsValid == false)
+ if (!paramsValid)
{
return;
}
diff --git a/redfish-core/ut/configfile_test.cpp b/redfish-core/ut/configfile_test.cpp
index 9f6bc13..7e9c785 100644
--- a/redfish-core/ut/configfile_test.cpp
+++ b/redfish-core/ut/configfile_test.cpp
@@ -49,7 +49,7 @@
{
crow::Response res;
- const std::string fileName = "";
+ const std::string fileName;
EXPECT_FALSE(isValidConfigFileName(fileName, res));
}
TEST(ConfigFileTest, FileNameSlash)