PEL: Fix more cppcheck warnings
This is the second of two commits to fix most of the cppcheck warnings
in the PEL code. It doesn't fix all of them because some are false
positives and some are just suggestions.
It's broken up into two commits to make them smaller.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: Id9f462386df85fd25d09529d6b410115ff4ccba8
diff --git a/extensions/openpower-pels/sbe_ffdc_handler.cpp b/extensions/openpower-pels/sbe_ffdc_handler.cpp
index 9c75183..7d02693 100644
--- a/extensions/openpower-pels/sbe_ffdc_handler.cpp
+++ b/extensions/openpower-pels/sbe_ffdc_handler.cpp
@@ -234,6 +234,7 @@
pdf.format = openpower::pels::UserDataFormat::text;
pdf.version = 0x01;
pdf.fd = pelDataFile.getFd();
+ pdf.subType = 0;
ffdcFiles.push_back(pdf);
paths.push_back(pelDataFile.getPath());
diff --git a/extensions/openpower-pels/service_indicators.cpp b/extensions/openpower-pels/service_indicators.cpp
index fca9f27..d2a9b43 100644
--- a/extensions/openpower-pels/service_indicators.cpp
+++ b/extensions/openpower-pels/service_indicators.cpp
@@ -197,7 +197,6 @@
const std::vector<std::string>& locationCodes) const
{
std::vector<std::string> paths;
- std::string inventoryPath;
for (const auto& locCode : locationCodes)
{
diff --git a/extensions/openpower-pels/src.cpp b/extensions/openpower-pels/src.cpp
index 1a5acf7..4c076c9 100644
--- a/extensions/openpower-pels/src.cpp
+++ b/extensions/openpower-pels/src.cpp
@@ -126,8 +126,7 @@
std::optional<std::string> getPythonJSON(std::vector<std::string>& hexwords,
uint8_t creatorID)
{
- PyObject *pName, *pModule, *pDict, *pFunc, *pArgs, *pResult, *pBytes,
- *eType, *eValue, *eTraceback, *pKey;
+ PyObject *pName, *pModule, *eType, *eValue, *eTraceback;
std::string pErrStr;
std::string module = getNumberString("%c", tolower(creatorID)) + "src";
pName = PyUnicode_FromString(
@@ -162,9 +161,9 @@
std::unique_ptr<PyObject, decltype(&pyDecRef)> modPtr(pModule,
&pyDecRef);
std::string funcToCall = "parseSRCToJson";
- pKey = PyUnicode_FromString(funcToCall.c_str());
+ PyObject* pKey = PyUnicode_FromString(funcToCall.c_str());
std::unique_ptr<PyObject, decltype(&pyDecRef)> keyPtr(pKey, &pyDecRef);
- pDict = PyModule_GetDict(pModule);
+ PyObject* pDict = PyModule_GetDict(pModule);
Py_INCREF(pDict);
if (!PyDict_Contains(pDict, pKey))
{
@@ -177,12 +176,12 @@
entry("PARSER_MODULE=%s", module.c_str()));
return std::nullopt;
}
- pFunc = PyDict_GetItemString(pDict, funcToCall.c_str());
+ PyObject* pFunc = PyDict_GetItemString(pDict, funcToCall.c_str());
Py_DECREF(pDict);
Py_INCREF(pFunc);
if (PyCallable_Check(pFunc))
{
- pArgs = PyTuple_New(9);
+ PyObject* pArgs = PyTuple_New(9);
std::unique_ptr<PyObject, decltype(&pyDecRef)> argPtr(pArgs,
&pyDecRef);
for (size_t i = 0; i < 9; i++)
@@ -198,13 +197,14 @@
PyTuple_SetItem(pArgs, i, Py_BuildValue("s", "00000000"));
}
}
- pResult = PyObject_CallObject(pFunc, pArgs);
+ PyObject* pResult = PyObject_CallObject(pFunc, pArgs);
Py_DECREF(pFunc);
if (pResult)
{
std::unique_ptr<PyObject, decltype(&pyDecRef)> resPtr(
pResult, &pyDecRef);
- pBytes = PyUnicode_AsEncodedString(pResult, "utf-8", "~E~");
+ PyObject* pBytes =
+ PyUnicode_AsEncodedString(pResult, "utf-8", "~E~");
std::unique_ptr<PyObject, decltype(&pyDecRef)> pyBytePtr(
pBytes, &pyDecRef);
const char* output = PyBytes_AS_STRING(pBytes);
diff --git a/extensions/openpower-pels/tools/peltool.cpp b/extensions/openpower-pels/tools/peltool.cpp
index c1c6da6..a45be58 100644
--- a/extensions/openpower-pels/tools/peltool.cpp
+++ b/extensions/openpower-pels/tools/peltool.cpp
@@ -75,10 +75,11 @@
uint64_t fileNameToTimestamp(const std::string& fileName)
{
std::string token = fileName.substr(0, fileName.find("_"));
- int i = 0;
uint64_t bcdTime = 0;
if (token.length() >= 14)
{
+ int i = 0;
+
try
{
auto tmp = std::stoul(token.substr(i, 2), 0, 16);
@@ -297,9 +298,7 @@
const std::vector<std::string>& plugins, bool hexDump,
bool archive)
{
- std::size_t found;
std::string val;
- char tmpValStr[50];
std::string listStr;
char name[51];
sprintf(name, "/%.2X%.2X%.2X%.2X%.2X%.2X%.2X%.2X_%.8X",
@@ -410,6 +409,7 @@
jsonInsert(listStr, "Subsystem", subsystem, 2);
// commit time
+ char tmpValStr[50];
sprintf(tmpValStr, "%02X/%02X/%02X%02X %02X:%02X:%02X",
pel.privateHeader().commitTimestamp().month,
pel.privateHeader().commitTimestamp().day,
@@ -431,7 +431,7 @@
"0x%X", pel.privateHeader().header().componentID),
2);
- found = listStr.rfind(",");
+ auto found = listStr.rfind(",");
if (found != std::string::npos)
{
listStr.replace(found, 1, "");
@@ -484,9 +484,10 @@
}
// Sort the pairs based on second time parameter
- std::sort(PELs.begin(), PELs.end(), [](auto& left, auto& right) {
- return left.second < right.second;
- });
+ std::sort(PELs.begin(), PELs.end(),
+ [](const auto& left, const auto& right) {
+ return left.second < right.second;
+ });
bool foundPEL = false;
@@ -557,7 +558,7 @@
{
std::transform(pelID.begin(), pelID.end(), pelID.begin(), toupper);
- if (pelID.find("0X") == 0)
+ if (pelID.starts_with("0X"))
{
pelID.erase(0, 2);
}
@@ -626,7 +627,7 @@
std::transform(pelID.begin(), pelID.end(), pelID.begin(), toupper);
- if (pelID.find("0X") == 0)
+ if (pelID.starts_with("0X"))
{
pelID.erase(0, 2);
}
diff --git a/extensions/openpower-pels/user_data_json.cpp b/extensions/openpower-pels/user_data_json.cpp
index 1c1c966..da8b233 100644
--- a/extensions/openpower-pels/user_data_json.cpp
+++ b/extensions/openpower-pels/user_data_json.cpp
@@ -254,8 +254,7 @@
const std::vector<uint8_t>& data,
uint8_t creatorID)
{
- PyObject *pName, *pModule, *pDict, *pFunc, *pArgs, *pData, *pResult,
- *pBytes, *eType, *eValue, *eTraceback, *pKey;
+ PyObject *pName, *pModule, *eType, *eValue, *eTraceback, *pKey;
std::string pErrStr;
std::string module = getNumberString("%c", tolower(creatorID)) +
getNumberString("%04x", componentID);
@@ -293,7 +292,7 @@
std::string funcToCall = "parseUDToJson";
pKey = PyUnicode_FromString(funcToCall.c_str());
std::unique_ptr<PyObject, decltype(&pyDecRef)> keyPtr(pKey, &pyDecRef);
- pDict = PyModule_GetDict(pModule);
+ PyObject* pDict = PyModule_GetDict(pModule);
Py_INCREF(pDict);
if (!PyDict_Contains(pDict, pKey))
{
@@ -307,30 +306,31 @@
entry("DATA_LENGTH=%lu\n", data.size()));
return std::nullopt;
}
- pFunc = PyDict_GetItemString(pDict, funcToCall.c_str());
+ PyObject* pFunc = PyDict_GetItemString(pDict, funcToCall.c_str());
Py_DECREF(pDict);
Py_INCREF(pFunc);
if (PyCallable_Check(pFunc))
{
auto ud = data.data();
- pArgs = PyTuple_New(3);
+ PyObject* pArgs = PyTuple_New(3);
std::unique_ptr<PyObject, decltype(&pyDecRef)> argPtr(pArgs,
&pyDecRef);
PyTuple_SetItem(pArgs, 0,
PyLong_FromUnsignedLong((unsigned long)subType));
PyTuple_SetItem(pArgs, 1,
PyLong_FromUnsignedLong((unsigned long)version));
- pData = PyMemoryView_FromMemory(
+ PyObject* pData = PyMemoryView_FromMemory(
reinterpret_cast<char*>(const_cast<unsigned char*>(ud)),
data.size(), PyBUF_READ);
PyTuple_SetItem(pArgs, 2, pData);
- pResult = PyObject_CallObject(pFunc, pArgs);
+ PyObject* pResult = PyObject_CallObject(pFunc, pArgs);
Py_DECREF(pFunc);
if (pResult)
{
std::unique_ptr<PyObject, decltype(&pyDecRef)> resPtr(
pResult, &pyDecRef);
- pBytes = PyUnicode_AsEncodedString(pResult, "utf-8", "~E~");
+ PyObject* pBytes =
+ PyUnicode_AsEncodedString(pResult, "utf-8", "~E~");
std::unique_ptr<PyObject, decltype(&pyDecRef)> pyBytePtr(
pBytes, &pyDecRef);
const char* output = PyBytes_AS_STRING(pBytes);
diff --git a/extensions/openpower-pels/user_header.cpp b/extensions/openpower-pels/user_header.cpp
index b2365a9..49da47a 100644
--- a/extensions/openpower-pels/user_header.cpp
+++ b/extensions/openpower-pels/user_header.cpp
@@ -103,12 +103,13 @@
{
bool mfgSevStatus = false;
bool mfgActionFlagStatus = false;
- std::optional<uint8_t> sev = std::nullopt;
- uint16_t val = 0;
// Get the mfg severity & action flags
if (entry.mfgSeverity || entry.mfgActionFlags)
{
+ std::optional<uint8_t> sev = std::nullopt;
+ uint16_t val = 0;
+
if (entry.mfgSeverity)
{
// Find the mf severity possibly dependent on the system type.
@@ -161,7 +162,6 @@
{
// Either someone screwed up the message registry
// or getSystemNames failed.
- std::string types;
log<level::ERR>(
"Failed finding the severity in the message registry",
phosphor::logging::entry("ERROR=%s",
diff --git a/test/openpower-pels/fru_identity_test.cpp b/test/openpower-pels/fru_identity_test.cpp
index 28a8e98..24ac5fe 100644
--- a/test/openpower-pels/fru_identity_test.cpp
+++ b/test/openpower-pels/fru_identity_test.cpp
@@ -89,7 +89,7 @@
EXPECT_THROW(FRUIdentity fru{stream}, std::out_of_range);
}
-void testHWCallout(const std::string& pn, const std::string ccin,
+void testHWCallout(const std::string& pn, const std::string& ccin,
const std::string& sn, const std::string& expectedPN,
const std::string& expectedCCIN,
const std::string& expectedSN)
diff --git a/test/openpower-pels/mtms_test.cpp b/test/openpower-pels/mtms_test.cpp
index b9c0929..b0f0130 100644
--- a/test/openpower-pels/mtms_test.cpp
+++ b/test/openpower-pels/mtms_test.cpp
@@ -32,12 +32,12 @@
MTMS mtms{tm, sn};
- std::array<uint8_t, 8> t{'T', 'T', 'T', 'T', '-', 'M', 'M', 'M'};
+ const std::array<uint8_t, 8> t{'T', 'T', 'T', 'T', '-', 'M', 'M', 'M'};
EXPECT_EQ(t, mtms.machineTypeAndModelRaw());
EXPECT_EQ("TTTT-MMM", mtms.machineTypeAndModel());
- std::array<uint8_t, 12> s{'1', '2', '3', '4', '5', '6',
- '7', '8', '9', 'A', 'B', 'C'};
+ const std::array<uint8_t, 12> s{'1', '2', '3', '4', '5', '6',
+ '7', '8', '9', 'A', 'B', 'C'};
EXPECT_EQ(s, mtms.machineSerialNumberRaw());
EXPECT_EQ("123456789ABC", mtms.machineSerialNumber());
}
@@ -49,11 +49,11 @@
MTMS mtms{tm, sn};
- std::array<uint8_t, 8> t{'T', 'T', 'T', 'T', '-', 'M', 'M', 'M'};
+ const std::array<uint8_t, 8> t{'T', 'T', 'T', 'T', '-', 'M', 'M', 'M'};
EXPECT_EQ(t, mtms.machineTypeAndModelRaw());
- std::array<uint8_t, 12> s{'1', '2', '3', '4', '5', '6',
- '7', '8', '9', 'A', 'B', 'C'};
+ const std::array<uint8_t, 12> s{'1', '2', '3', '4', '5', '6',
+ '7', '8', '9', 'A', 'B', 'C'};
EXPECT_EQ(s, mtms.machineSerialNumberRaw());
}
@@ -64,11 +64,12 @@
MTMS mtms{tm, sn};
- std::array<uint8_t, 8> t{'T', 'T', 'T', 'T', 0, 0, 0, 0};
+ const std::array<uint8_t, 8> t{'T', 'T', 'T', 'T', 0, 0, 0, 0};
EXPECT_EQ(t, mtms.machineTypeAndModelRaw());
EXPECT_EQ("TTTT", mtms.machineTypeAndModel());
- std::array<uint8_t, 12> s{'1', '2', '3', '4', 0, 0, 0, 0, 0, 0, 0, 0};
+ const std::array<uint8_t, 12> s{'1', '2', '3', '4', 0, 0,
+ 0, 0, 0, 0, 0, 0};
EXPECT_EQ(s, mtms.machineSerialNumberRaw());
EXPECT_EQ("1234", mtms.machineSerialNumber());
}
diff --git a/test/openpower-pels/pel_manager_test.cpp b/test/openpower-pels/pel_manager_test.cpp
index fc77a5e..00c3fd6 100644
--- a/test/openpower-pels/pel_manager_test.cpp
+++ b/test/openpower-pels/pel_manager_test.cpp
@@ -721,7 +721,7 @@
}
catch (
const sdbusplus::xyz::openbmc_project::Common::Error::InvalidArgument&
- e)
+ ex)
{
ADD_FAILURE() << "PELs should have all been found";
}
diff --git a/test/openpower-pels/pel_test.cpp b/test/openpower-pels/pel_test.cpp
index 352e0ed..d5cd373 100644
--- a/test/openpower-pels/pel_test.cpp
+++ b/test/openpower-pels/pel_test.cpp
@@ -259,14 +259,12 @@
EXPECT_EQ(pel.size(), 16384);
// Make sure that there are still 2 UD sections.
- size_t udCount = 0;
- for (const auto& section : pel.optionalSections())
- {
- if (section->header().id == static_cast<uint16_t>(SectionID::userData))
- {
- udCount++;
- }
- }
+ const auto& optSections = pel.optionalSections();
+ auto udCount = std::count_if(
+ optSections.begin(), optSections.end(), [](const auto& section) {
+ return section->header().id ==
+ static_cast<uint16_t>(SectionID::userData);
+ });
EXPECT_EQ(udCount, 2); // AD section and sysInfo section
}
diff --git a/test/openpower-pels/repository_test.cpp b/test/openpower-pels/repository_test.cpp
index 73a24d8..7502dfa 100644
--- a/test/openpower-pels/repository_test.cpp
+++ b/test/openpower-pels/repository_test.cpp
@@ -659,8 +659,8 @@
// there so we can check they are removed after the prune.
for (uint32_t i = 1; i < 5; i++)
{
- Repository::LogID id{Repository::LogID::Pel{i}};
- EXPECT_TRUE(repo.getPELAttributes(id));
+ Repository::LogID logID{Repository::LogID::Pel{i}};
+ EXPECT_TRUE(repo.getPELAttributes(logID));
}
// Prune down to 15%/30%/15%/30% = 90% total
@@ -677,8 +677,8 @@
// each type, were removed.
for (uint32_t i = 1; i < 5; i++)
{
- Repository::LogID id{Repository::LogID::Pel{i}};
- EXPECT_FALSE(repo.getPELAttributes(id));
+ Repository::LogID logID{Repository::LogID::Pel{i}};
+ EXPECT_FALSE(repo.getPELAttributes(logID));
// Make sure the corresponding OpenBMC event log ID which is
// 500 + the PEL ID is in the list.
@@ -710,8 +710,8 @@
// get pruned below we'll know they were removed.
for (uint32_t i = 1; i <= 20; i++)
{
- Repository::LogID id{Repository::LogID::Pel{i}};
- EXPECT_TRUE(repo.getPELAttributes(id));
+ Repository::LogID logID{Repository::LogID::Pel{i}};
+ EXPECT_TRUE(repo.getPELAttributes(logID));
}
auto IDs = repo.prune(id);
@@ -728,8 +728,8 @@
// Can no longer find the oldest 20 PELs.
for (uint32_t i = 1; i <= 20; i++)
{
- Repository::LogID id{Repository::LogID::Pel{i}};
- EXPECT_FALSE(repo.getPELAttributes(id));
+ Repository::LogID logID{Repository::LogID::Pel{i}};
+ EXPECT_FALSE(repo.getPELAttributes(logID));
EXPECT_TRUE(std::find(IDs.begin(), IDs.end(), 500 + i) != IDs.end());
}
}
@@ -760,11 +760,11 @@
auto idToDelete = pel->obmcLogID();
repo.add(pel);
- if (0 == i)
+ if (1 == i)
{
repo.setPELHMCTransState(pel->id(), TransmissionState::acked);
}
- else if (1 == i)
+ else if (2 == i)
{
repo.setPELHostTransState(pel->id(), TransmissionState::acked);
}