Enable readability checks
clang-tidy readability checks are overall a good thing, and help us to
write consistent and readable code, even if it doesn't change the
result.
All changes done by the robot.
Tested: Code compiles, inspection only (changes made by robot)
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iee4a0c74a11eef9f158f0044eae675ebc518b549
diff --git a/.clang-tidy b/.clang-tidy
index dfca83a..d892bdd 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -276,10 +276,18 @@
readability-delete-null-pointer,
readability-deleted-default,
readability-else-after-return,
+readability-function-size,
+readability-identifier-naming,
readability-implicit-bool-conversion,
+readability-inconsistent-declaration-parameter-name,
+readability-isolate-declaration,
readability-make-member-function-const,
+readability-misleading-indentation,
+readability-misplaced-array-index,
readability-named-parameter,
readability-non-const-parameter,
+readability-qualified-auto,
+readability-redundant-access-specifiers,
readability-redundant-control-flow,
readability-redundant-declaration,
readability-redundant-function-ptr-dereference,
@@ -288,8 +296,13 @@
readability-redundant-smartptr-get,
readability-redundant-string-cstr,
readability-redundant-string-init,
+readability-simplify-boolean-expr,
+readability-simplify-subscript-expr,
readability-static-accessed-through-instance,
-readability-identifier-naming,
+readability-static-definition-in-anonymous-namespace,
+readability-string-compare,
+readability-suspicious-call-argument,
+readability-uniqueptr-delete-release,
readability-uppercase-literal-suffix'
WarningsAsErrors: '*'
@@ -301,4 +314,3 @@
- { key: readability-identifier-naming.ParameterCase, value: camelBack }
- { key: readability-identifier-naming.NamespaceCase, value: lower_case }
- { key: readability-identifier-naming.StructCase, value: CamelCase }
-
diff --git a/include/json_html_serializer.hpp b/include/json_html_serializer.hpp
index 022ee13..320b5fd1 100644
--- a/include/json_html_serializer.hpp
+++ b/include/json_html_serializer.hpp
@@ -325,7 +325,7 @@
}
// use a pointer to fill the buffer
- auto bufferPtr = begin(numberbuffer);
+ auto* bufferPtr = begin(numberbuffer);
const bool isNegative = std::is_same<NumberType, int64_t>::value &&
!(number >= 0); // see issue #755
@@ -364,18 +364,23 @@
{
const auto digitsIndex = static_cast<unsigned>((absValue % 100));
absValue /= 100;
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
*(--bufferPtr) = digitsTo99[digitsIndex][1];
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
*(--bufferPtr) = digitsTo99[digitsIndex][0];
}
if (absValue >= 10)
{
const auto digitsIndex = static_cast<unsigned>(absValue);
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
*(--bufferPtr) = digitsTo99[digitsIndex][1];
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
*(--bufferPtr) = digitsTo99[digitsIndex][0];
}
else
{
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
*(--bufferPtr) = static_cast<char>('0' + absValue);
}
@@ -418,7 +423,7 @@
return;
}
- const auto end =
+ const std::array<char, 64>::iterator end =
std::remove(numberbuffer.begin(), numberbuffer.begin() + len, ',');
std::fill(end, numberbuffer.end(), '\0');
diff --git a/include/login_routes.hpp b/include/login_routes.hpp
index 1087b0b..abcdaee 100644
--- a/include/login_routes.hpp
+++ b/include/login_routes.hpp
@@ -242,7 +242,7 @@
.methods(boost::beast::http::verb::post)(
[](const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) {
- auto& session = req.session;
+ const auto& session = req.session;
if (session != nullptr)
{
asyncResp->res.jsonValue = {
diff --git a/include/ssl_key_handler.hpp b/include/ssl_key_handler.hpp
index 1213758..431dd84 100644
--- a/include/ssl_key_handler.hpp
+++ b/include/ssl_key_handler.hpp
@@ -25,15 +25,11 @@
// Trust chain related errors.`
inline bool isTrustChainError(int errnum)
{
- if ((errnum == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) ||
- (errnum == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN) ||
- (errnum == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY) ||
- (errnum == X509_V_ERR_CERT_UNTRUSTED) ||
- (errnum == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE))
- {
- return true;
- }
- return false;
+ return (errnum == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) ||
+ (errnum == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN) ||
+ (errnum == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY) ||
+ (errnum == X509_V_ERR_CERT_UNTRUSTED) ||
+ (errnum == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE);
}
inline bool validateCertificate(X509* const cert)
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index e6e694f..7aeacd6 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -95,7 +95,7 @@
std::span<const MessageEntry>::iterator messageIt =
std::find_if(registry.begin(), registry.end(),
[&messageKey](const MessageEntry& messageEntry) {
- return messageKey.compare(messageEntry.first) == 0;
+ return messageKey == messageEntry.first;
});
if (messageIt != registry.end())
{
@@ -921,11 +921,7 @@
bool isSubscriptionExist(const std::string& id)
{
auto obj = subscriptionsMap.find(id);
- if (obj == subscriptionsMap.end())
- {
- return false;
- }
- return true;
+ return obj != subscriptionsMap.end();
}
void deleteSubscription(const std::string& id)
diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp
index 9aac1dc..b688576 100644
--- a/redfish-core/include/utils/json_utils.hpp
+++ b/redfish-core/include/utils/json_utils.hpp
@@ -430,7 +430,7 @@
result = details::unpackValue<nlohmann::json>(item.value(), key,
res, j) &&
result;
- if (result == false)
+ if (!result)
{
return result;
}
@@ -475,7 +475,7 @@
for (PerUnpack& perUnpack : toUnpack)
{
- if (perUnpack.complete == false)
+ if (!perUnpack.complete)
{
bool isOptional = std::visit(
[](auto&& val) {
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 17f5fd3..5cd70bc 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -1727,8 +1727,7 @@
const std::pair<sdbusplus::message::object_path,
dbus::utility::DBusInteracesMap>&
user) {
- return accountName.compare(user.first.filename()) ==
- 0;
+ return accountName == user.first.filename();
});
if (userIt == users.end())
diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp
index ba39140..134ef24 100644
--- a/redfish-core/lib/event_service.hpp
+++ b/redfish-core/lib/event_service.hpp
@@ -429,7 +429,7 @@
registry.begin(), registry.end(),
[&id](const redfish::registries::MessageEntry&
messageEntry) {
- return id.compare(messageEntry.first) == 0;
+ return id == messageEntry.first;
}))
{
validId = true;
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index 629db47..8a0bd3a 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -434,7 +434,7 @@
{
if (propertyMap.first == "Status")
{
- auto status = std::get_if<std::string>(
+ const auto* status = std::get_if<std::string>(
&propertyMap.second);
if (status == nullptr)
{
@@ -453,7 +453,7 @@
{
if (propertyMap.first == "Size")
{
- auto sizePtr =
+ const auto* sizePtr =
std::get_if<uint64_t>(&propertyMap.second);
if (sizePtr == nullptr)
{
@@ -988,7 +988,7 @@
return;
}
- for (auto& pathStr : subtreePath)
+ for (const auto& pathStr : subtreePath)
{
if (pathStr.find("PostCode") != std::string::npos)
{
diff --git a/redfish-core/lib/power.hpp b/redfish-core/lib/power.hpp
index 694d29e..eaf7e6c 100644
--- a/redfish-core/lib/power.hpp
+++ b/redfish-core/lib/power.hpp
@@ -172,8 +172,7 @@
std::string interfaceChassisName =
chassis.substr(lastPos + 1, len);
- if (interfaceChassisName.compare(
- sensorAsyncResp->chassisId) == 0)
+ if (interfaceChassisName == sensorAsyncResp->chassisId)
{
found = true;
break;
@@ -231,7 +230,7 @@
dbus::utility::DbusVariantType>&
property : properties)
{
- if (property.first.compare("Scale") == 0)
+ if (property.first == "Scale")
{
const int64_t* i =
std::get_if<int64_t>(&property.second);
@@ -241,7 +240,7 @@
scale = *i;
}
}
- else if (property.first.compare("PowerCap") == 0)
+ else if (property.first == "PowerCap")
{
const double* d =
std::get_if<double>(&property.second);
@@ -263,8 +262,7 @@
powerCap = *u;
}
}
- else if (property.first.compare("PowerCapEnable") ==
- 0)
+ else if (property.first == "PowerCapEnable")
{
const bool* b =
std::get_if<bool>(&property.second);
diff --git a/redfish-core/lib/redfish_util.hpp b/redfish-core/lib/redfish_util.hpp
index a51aafd..a42d00d 100644
--- a/redfish-core/lib/redfish_util.hpp
+++ b/redfish-core/lib/redfish_util.hpp
@@ -120,7 +120,7 @@
// is unitsName end with ".socket"
std::string unitNameEnd = unitName.substr(lastCharPos);
- if (unitNameEnd.compare(".socket") != 0)
+ if (unitNameEnd != ".socket")
{
continue;
}
diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp
index dd2d752..3139635 100644
--- a/redfish-core/lib/sensors.hpp
+++ b/redfish-core/lib/sensors.hpp
@@ -965,7 +965,7 @@
std::string sensorNameLower =
boost::algorithm::to_lower_copy(sensorName);
- if (sensorName.compare("total_power") == 0)
+ if (sensorName == "total_power")
{
sensorJson["@odata.type"] = "#Power.v1_0_0.PowerControl";
// Put multiple "sensors" into a single PowerControl, so have
@@ -1054,13 +1054,13 @@
for (const std::tuple<const char*, const char*,
nlohmann::json::json_pointer>& p : properties)
{
- for (auto& [interface, values] : interfacesDict)
+ for (const auto& [interface, values] : interfacesDict)
{
if (interface != std::get<0>(p))
{
continue;
}
- for (auto& [valueName, valueVariant] : values)
+ for (const auto& [valueName, valueVariant] : values)
{
if (valueName != std::get<1>(p))
{
@@ -2167,7 +2167,7 @@
// Store value in Power Supply Inventory Items
for (InventoryItem& inventoryItem : *inventoryItems)
{
- if (inventoryItem.isPowerSupply == true)
+ if (inventoryItem.isPowerSupply)
{
inventoryItem.powerSupplyEfficiencyPercent =
static_cast<int>(value);
@@ -2551,7 +2551,7 @@
}
else if (sensorType == "power")
{
- if (sensorName.compare("total_power") == 0)
+ if (sensorName == "total_power")
{
fieldName = "PowerControl";
}
diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp
index 0d1e12d..ec4db21 100644
--- a/redfish-core/lib/systems.hpp
+++ b/redfish-core/lib/systems.hpp
@@ -77,11 +77,11 @@
{
BMCWEB_LOG_DEBUG << "Cpu Present: " << isCpuPresent;
- if (isCpuPresent == true)
+ if (isCpuPresent)
{
nlohmann::json& procCount =
aResp->res.jsonValue["ProcessorSummary"]["Count"];
- auto procCountPtr =
+ auto* procCountPtr =
procCount.get_ptr<nlohmann::json::number_integer_t*>();
if (procCountPtr != nullptr)
{