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/include/ibm/locks.hpp b/include/ibm/locks.hpp
index f6719c7..b897fa7 100644
--- a/include/ibm/locks.hpp
+++ b/include/ibm/locks.hpp
@@ -317,20 +317,16 @@
BMCWEB_LOG_DEBUG << "Not a Valid request id";
return std::make_pair(false, status);
}
- else
+ // Validation passed, check if all the locks are owned by the
+ // requesting HMC
+ auto status2 = isItMyLock(p, ids);
+ if (status2.first)
{
- // Validation passed, check if all the locks are owned by the
- // requesting HMC
- auto status = isItMyLock(p, ids);
- if (status.first)
- {
- // The current hmc owns all the locks, so we can release
- // them
- releaseLock(p);
- }
- return std::make_pair(true, status);
+ // The current hmc owns all the locks, so we can release
+ // them
+ releaseLock(p);
}
- return std::make_pair(false, status);
+ return std::make_pair(true, status);
}
inline RcAcquireLock Lock::acquireLock(const LockRequests lockRequestStructure)
@@ -358,19 +354,14 @@
BMCWEB_LOG_DEBUG << "There is a conflict within itself";
return std::make_pair(true, std::make_pair(status, 1));
}
- else
- {
- BMCWEB_LOG_DEBUG << "The request is not conflicting within itself";
+ BMCWEB_LOG_DEBUG << "The request is not conflicting within itself";
- // Need to check for conflict with the locktable entries.
+ // Need to check for conflict with the locktable entries.
- auto conflict = isConflictWithTable(multiRequest);
+ auto conflict = isConflictWithTable(multiRequest);
- BMCWEB_LOG_DEBUG << "Done with checking conflict with the locktable";
- return std::make_pair(false, conflict);
- }
-
- return std::make_pair(true, std::make_pair(true, 1));
+ BMCWEB_LOG_DEBUG << "Done with checking conflict with the locktable";
+ return std::make_pair(false, conflict);
}
inline void Lock::releaseLock(const ListOfTransactionIds& refRids)
@@ -562,43 +553,38 @@
saveLocks();
return std::make_pair(false, transactionId);
}
+ BMCWEB_LOG_DEBUG
+ << "Lock table is not empty, check for conflict with lock table";
+ // Lock table is not empty, compare the lockrequest entries with
+ // the entries in the lock table
- else
+ for (const auto& lockRecord1 : refLockRequestStructure)
{
- BMCWEB_LOG_DEBUG
- << "Lock table is not empty, check for conflict with lock table";
- // Lock table is not empty, compare the lockrequest entries with
- // the entries in the lock table
-
- for (const auto& lockRecord1 : refLockRequestStructure)
+ for (const auto& map : lockTable)
{
- for (const auto& map : lockTable)
+ for (const auto& lockRecord2 : map.second)
{
- for (const auto& lockRecord2 : map.second)
+ bool status = isConflictRecord(lockRecord1, lockRecord2);
+ if (status)
{
- bool status = isConflictRecord(lockRecord1, lockRecord2);
- if (status)
- {
- return std::make_pair(
- true, std::make_pair(map.first, lockRecord2));
- }
+ return std::make_pair(
+ true, std::make_pair(map.first, lockRecord2));
}
}
}
-
- // Reached here, so no conflict with the locktable, so we are safe to
- // add the request records into the lock table
-
- // Lock table is empty, so we are safe to add the lockrecords
- // as there will be no conflict
- BMCWEB_LOG_DEBUG << " Adding elements into lock table";
- transactionId = generateTransactionId();
- lockTable.emplace(
- std::make_pair(transactionId, refLockRequestStructure));
-
- // save the lock in the persistent file
- saveLocks();
}
+
+ // Reached here, so no conflict with the locktable, so we are safe to
+ // add the request records into the lock table
+
+ // Lock table is empty, so we are safe to add the lockrecords
+ // as there will be no conflict
+ BMCWEB_LOG_DEBUG << " Adding elements into lock table";
+ transactionId = generateTransactionId();
+ lockTable.emplace(std::make_pair(transactionId, refLockRequestStructure));
+
+ // save the lock in the persistent file
+ saveLocks();
return std::make_pair(false, transactionId);
}
@@ -615,25 +601,22 @@
return false;
}
- else
+ BMCWEB_LOG_DEBUG
+ << "There are multiple lock requests coming in a single request";
+
+ // There are multiple requests a part of one request
+
+ for (uint32_t i = 0; i < refLockRequestStructure.size(); i++)
{
- BMCWEB_LOG_DEBUG
- << "There are multiple lock requests coming in a single request";
-
- // There are multiple requests a part of one request
-
- for (uint32_t i = 0; i < refLockRequestStructure.size(); i++)
+ for (uint32_t j = i + 1; j < refLockRequestStructure.size(); j++)
{
- for (uint32_t j = i + 1; j < refLockRequestStructure.size(); j++)
- {
- const LockRequest& p = refLockRequestStructure[i];
- const LockRequest& q = refLockRequestStructure[j];
- bool status = isConflictRecord(p, q);
+ const LockRequest& p = refLockRequestStructure[i];
+ const LockRequest& q = refLockRequestStructure[j];
+ bool status = isConflictRecord(p, q);
- if (status)
- {
- return true;
- }
+ if (status)
+ {
+ return true;
}
}
}
@@ -661,11 +644,6 @@
return false;
}
- else
- {
- return true;
- }
-
return true;
}
@@ -681,69 +659,64 @@
return false;
}
- else
+ uint32_t i = 0;
+ for (const auto& p : std::get<4>(refLockRecord1))
{
- uint32_t i = 0;
- for (const auto& p : std::get<4>(refLockRecord1))
+
+ // return conflict when any of them is try to lock all resources
+ // under the current resource level.
+ if (boost::equals(p.first, "LockAll") ||
+ boost::equals(std::get<4>(refLockRecord2)[i].first, "LockAll"))
{
+ BMCWEB_LOG_DEBUG
+ << "Either of the Comparing locks are trying to lock all "
+ "resources under the current resource level";
+ return true;
+ }
- // return conflict when any of them is try to lock all resources
- // under the current resource level.
- if (boost::equals(p.first, "LockAll") ||
- boost::equals(std::get<4>(refLockRecord2)[i].first, "LockAll"))
+ // determine if there is a lock-all-with-same-segment-size.
+ // If the current segment sizes are the same,then we should fail.
+
+ if ((boost::equals(p.first, "LockSame") ||
+ boost::equals(std::get<4>(refLockRecord2)[i].first, "LockSame")) &&
+ (p.second == std::get<4>(refLockRecord2)[i].second))
+ {
+ return true;
+ }
+
+ // if segment lengths are not the same, it means two different locks
+ // So no conflict
+ if (p.second != std::get<4>(refLockRecord2)[i].second)
+ {
+ BMCWEB_LOG_DEBUG << "Segment lengths are not same";
+ BMCWEB_LOG_DEBUG << "Segment 1 length : " << p.second;
+ BMCWEB_LOG_DEBUG << "Segment 2 length : "
+ << std::get<4>(refLockRecord2)[i].second;
+ return false;
+ }
+
+ // compare segment data
+
+ for (uint32_t i = 0; i < p.second; i++)
+ {
+ // if the segment data is different , then the locks is on a
+ // different resource So no conflict between the lock
+ // records.
+ // BMC is little endian , but the resourceID is formed by
+ // the Management Console in such a way that, the first byte
+ // from the MSB Position corresponds to the First Segment
+ // data. Therefore we need to convert the in-comming
+ // resourceID into Big Endian before processing further.
+ if (!(checkByte(
+ boost::endian::endian_reverse(std::get<3>(refLockRecord1)),
+ boost::endian::endian_reverse(std::get<3>(refLockRecord2)),
+ i)))
{
- BMCWEB_LOG_DEBUG
- << "Either of the Comparing locks are trying to lock all "
- "resources under the current resource level";
- return true;
- }
-
- // determine if there is a lock-all-with-same-segment-size.
- // If the current segment sizes are the same,then we should fail.
-
- if ((boost::equals(p.first, "LockSame") ||
- boost::equals(std::get<4>(refLockRecord2)[i].first,
- "LockSame")) &&
- (p.second == std::get<4>(refLockRecord2)[i].second))
- {
- return true;
- }
-
- // if segment lengths are not the same, it means two different locks
- // So no conflict
- if (p.second != std::get<4>(refLockRecord2)[i].second)
- {
- BMCWEB_LOG_DEBUG << "Segment lengths are not same";
- BMCWEB_LOG_DEBUG << "Segment 1 length : " << p.second;
- BMCWEB_LOG_DEBUG << "Segment 2 length : "
- << std::get<4>(refLockRecord2)[i].second;
return false;
}
-
- // compare segment data
-
- for (uint32_t i = 0; i < p.second; i++)
- {
- // if the segment data is different , then the locks is on a
- // different resource So no conflict between the lock
- // records.
- // BMC is little endian , but the resourceID is formed by
- // the Management Console in such a way that, the first byte
- // from the MSB Position corresponds to the First Segment
- // data. Therefore we need to convert the in-comming
- // resourceID into Big Endian before processing further.
- if (!(checkByte(boost::endian::endian_reverse(
- std::get<3>(refLockRecord1)),
- boost::endian::endian_reverse(
- std::get<3>(refLockRecord2)),
- i)))
- {
- return false;
- }
- }
-
- ++i;
}
+
+ ++i;
}
return false;