Make build on clang
And support clang-tidy rules. The changes are pretty minimal, and were
all done by the clang robot.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I29501aa56de1cd63cda233e06a07641458f89345
diff --git a/src/EntityManager.cpp b/src/EntityManager.cpp
index b36610b..490c0f5 100644
--- a/src/EntityManager.cpp
+++ b/src/EntityManager.cpp
@@ -47,11 +47,11 @@
constexpr const char* lastConfiguration = "/tmp/configuration/last.json";
constexpr const char* currentConfiguration = "/var/configuration/system.json";
constexpr const char* globalSchema = "global.json";
-constexpr const int32_t MAX_MAPPER_DEPTH = 0;
+constexpr const int32_t maxMapperDepth = 0;
-constexpr const bool DEBUG = false;
+constexpr const bool debug = false;
-struct cmp_str
+struct CmpStr
{
bool operator()(const char* a, const char* b) const
{
@@ -69,13 +69,13 @@
FOUND,
MATCH_ONE
};
-const static boost::container::flat_map<const char*, probe_type_codes, cmp_str>
- PROBE_TYPES{{{"FALSE", probe_type_codes::FALSE_T},
- {"TRUE", probe_type_codes::TRUE_T},
- {"AND", probe_type_codes::AND},
- {"OR", probe_type_codes::OR},
- {"FOUND", probe_type_codes::FOUND},
- {"MATCH_ONE", probe_type_codes::MATCH_ONE}}};
+const static boost::container::flat_map<const char*, probe_type_codes, CmpStr>
+ probeTypes{{{"FALSE", probe_type_codes::FALSE_T},
+ {"TRUE", probe_type_codes::TRUE_T},
+ {"AND", probe_type_codes::AND},
+ {"OR", probe_type_codes::OR},
+ {"FOUND", probe_type_codes::FOUND},
+ {"MATCH_ONE", probe_type_codes::MATCH_ONE}}};
static constexpr std::array<const char*, 6> settableInterfaces = {
"FanProfile", "Pid", "Pid.Zone", "Stepwise", "Thresholds", "Polling"};
@@ -99,13 +99,13 @@
inventory;
// todo: pass this through nicer
-std::shared_ptr<sdbusplus::asio::connection> SYSTEM_BUS;
+std::shared_ptr<sdbusplus::asio::connection> systemBus;
static nlohmann::json lastJson;
boost::asio::io_context io;
-const std::regex ILLEGAL_DBUS_PATH_REGEX("[^A-Za-z0-9_.]");
-const std::regex ILLEGAL_DBUS_MEMBER_REGEX("[^A-Za-z0-9_]");
+const std::regex illegalDbusPathRegex("[^A-Za-z0-9_.]");
+const std::regex illegalDbusMemberRegex("[^A-Za-z0-9_]");
void registerCallback(nlohmann::json& systemConfiguration,
sdbusplus::asio::object_server& objServer,
@@ -139,7 +139,7 @@
void getInterfaces(
const std::tuple<std::string, std::string, std::string>& call,
const std::vector<std::shared_ptr<PerformProbe>>& probeVector,
- std::shared_ptr<PerformScan> scan, size_t retries = 5)
+ const std::shared_ptr<PerformScan>& scan, size_t retries = 5)
{
if (!retries)
{
@@ -148,7 +148,7 @@
return;
}
- SYSTEM_BUS->async_method_call(
+ systemBus->async_method_call(
[call, scan, probeVector, retries](
boost::system::error_code& errc,
const boost::container::flat_map<std::string, BasicVariantType>&
@@ -175,7 +175,7 @@
std::get<0>(call), std::get<1>(call), "org.freedesktop.DBus.Properties",
"GetAll", std::get<2>(call));
- if constexpr (DEBUG)
+ if constexpr (debug)
{
std::cerr << __func__ << " " << __LINE__ << "\n";
}
@@ -185,7 +185,8 @@
// for the paths that own the interfaces passed in.
void findDbusObjects(std::vector<std::shared_ptr<PerformProbe>>&& probeVector,
boost::container::flat_set<std::string>&& interfaces,
- std::shared_ptr<PerformScan> scan, size_t retries = 5)
+ const std::shared_ptr<PerformScan>& scan,
+ size_t retries = 5)
{
// Filter out interfaces already obtained.
for (const auto& [path, probeInterfaces] : scan->dbusProbeObjects)
@@ -201,7 +202,7 @@
}
// find all connections in the mapper that expose a specific type
- SYSTEM_BUS->async_method_call(
+ systemBus->async_method_call(
[interfaces, probeVector{std::move(probeVector)}, scan,
retries](boost::system::error_code& ec,
const GetSubTreeType& interfaceSubtree) mutable {
@@ -273,10 +274,10 @@
},
"xyz.openbmc_project.ObjectMapper",
"/xyz/openbmc_project/object_mapper",
- "xyz.openbmc_project.ObjectMapper", "GetSubTree", "/", MAX_MAPPER_DEPTH,
+ "xyz.openbmc_project.ObjectMapper", "GetSubTree", "/", maxMapperDepth,
interfaces);
- if constexpr (DEBUG)
+ if constexpr (debug)
{
std::cerr << __func__ << " " << __LINE__ << "\n";
}
@@ -286,7 +287,7 @@
// When an interface passes a probe, also save its D-Bus path with it.
bool probeDbus(const std::string& interface,
const std::map<std::string, nlohmann::json>& matches,
- FoundDeviceT& devices, std::shared_ptr<PerformScan> scan,
+ FoundDeviceT& devices, const std::shared_ptr<PerformScan>& scan,
bool& foundProbe)
{
bool foundMatch = false;
@@ -323,7 +324,7 @@
}
if (deviceMatches)
{
- if constexpr (DEBUG)
+ if constexpr (debug)
{
std::cerr << "probeDBus: Found probe match on " << path << " "
<< interface << "\n";
@@ -338,7 +339,7 @@
// default probe entry point, iterates a list looking for specific types to
// call specific probe functions
bool probe(const std::vector<std::string>& probeCommand,
- std::shared_ptr<PerformScan> scan, FoundDeviceT& foundDevs)
+ const std::shared_ptr<PerformScan>& scan, FoundDeviceT& foundDevs)
{
const static std::regex command(R"(\((.*)\))");
std::smatch match;
@@ -352,9 +353,9 @@
{
bool foundProbe = false;
boost::container::flat_map<const char*, probe_type_codes,
- cmp_str>::const_iterator probeType;
+ CmpStr>::const_iterator probeType;
- for (probeType = PROBE_TYPES.begin(); probeType != PROBE_TYPES.end();
+ for (probeType = probeTypes.begin(); probeType != probeTypes.end();
++probeType)
{
if (probe.find(probeType->first) != std::string::npos)
@@ -434,7 +435,7 @@
// does a regex
std::map<std::string, nlohmann::json> dbusProbeMap =
json.get<std::map<std::string, nlohmann::json>>();
- auto findStart = probe.find("(");
+ auto findStart = probe.find('(');
if (findStart == std::string::npos)
{
return false;
@@ -460,9 +461,8 @@
ret = cur;
first = false;
}
- lastCommand = probeType != PROBE_TYPES.end()
- ? probeType->second
- : probe_type_codes::FALSE_T;
+ lastCommand = probeType != probeTypes.end() ? probeType->second
+ : probe_type_codes::FALSE_T;
}
// probe passed, but empty device
@@ -923,7 +923,7 @@
std::string dbusName = *name;
std::regex_replace(dbusName.begin(), dbusName.begin(),
- dbusName.end(), ILLEGAL_DBUS_MEMBER_REGEX, "_");
+ dbusName.end(), illegalDbusMemberRegex, "_");
std::shared_ptr<sdbusplus::asio::dbus_interface> interface =
createInterface(objServer, path + "/" + dbusName,
@@ -961,7 +961,7 @@
{
boardType = findBoardType->get<std::string>();
std::regex_replace(boardType.begin(), boardType.begin(),
- boardType.end(), ILLEGAL_DBUS_MEMBER_REGEX, "_");
+ boardType.end(), illegalDbusMemberRegex, "_");
}
else
{
@@ -972,9 +972,11 @@
std::string boardtypeLower = boost::algorithm::to_lower_copy(boardType);
std::regex_replace(boardKey.begin(), boardKey.begin(), boardKey.end(),
- ILLEGAL_DBUS_MEMBER_REGEX, "_");
- std::string boardName = "/xyz/openbmc_project/inventory/system/" +
- boardtypeLower + "/" + boardKey;
+ illegalDbusMemberRegex, "_");
+ std::string boardName = "/xyz/openbmc_project/inventory/system/";
+ boardName += boardtypeLower;
+ boardName += "/";
+ boardName += boardKey;
std::shared_ptr<sdbusplus::asio::dbus_interface> inventoryIface =
createInterface(objServer, boardName,
@@ -1044,8 +1046,7 @@
{
itemType = findType->get<std::string>();
std::regex_replace(itemType.begin(), itemType.begin(),
- itemType.end(), ILLEGAL_DBUS_PATH_REGEX,
- "_");
+ itemType.end(), illegalDbusPathRegex, "_");
}
else
{
@@ -1053,10 +1054,13 @@
}
std::string itemName = findName->get<std::string>();
std::regex_replace(itemName.begin(), itemName.begin(),
- itemName.end(), ILLEGAL_DBUS_MEMBER_REGEX, "_");
+ itemName.end(), illegalDbusMemberRegex, "_");
+ std::string ifacePath = boardName;
+ ifacePath += "/";
+ ifacePath += itemName;
std::shared_ptr<sdbusplus::asio::dbus_interface> itemIface =
- createInterface(objServer, boardName + "/" + itemName,
+ createInterface(objServer, ifacePath,
"xyz.openbmc_project.Configuration." + itemType,
boardKeyOrig);
@@ -1074,7 +1078,7 @@
{
std::shared_ptr<sdbusplus::asio::dbus_interface>
objectIface = createInterface(
- objServer, boardName + "/" + itemName,
+ objServer, ifacePath,
"xyz.openbmc_project.Configuration." + itemType +
"." + objectPair.key(),
boardKeyOrig);
@@ -1120,7 +1124,7 @@
std::shared_ptr<sdbusplus::asio::dbus_interface>
objectIface = createInterface(
- objServer, boardName + "/" + itemName,
+ objServer, ifacePath,
"xyz.openbmc_project.Configuration." +
itemType + "." + objectPair.key() +
std::to_string(index),
@@ -1230,7 +1234,7 @@
size_t hash = std::hash<std::string>{}(probeName + device.dump());
// hashes are hard to distinguish, use the
// non-hashed version if we want debug
- if constexpr (DEBUG)
+ if constexpr (debug)
{
return probeName + device.dump();
}
@@ -1268,7 +1272,7 @@
it = _configurations.erase(it);
continue;
}
- else if ((*findProbe).type() != nlohmann::json::value_t::array)
+ if ((*findProbe).type() != nlohmann::json::value_t::array)
{
probeCommand = nlohmann::json::array();
probeCommand.push_back(*findProbe);
@@ -1307,7 +1311,7 @@
std::list<size_t> indexes(foundDevices.size());
std::iota(indexes.begin(), indexes.end(), 1);
- size_t indexIdx = probeName.find("$");
+ size_t indexIdx = probeName.find('$');
bool hasTemplateName = (indexIdx != std::string::npos);
// copy over persisted configurations and make sure we remove
@@ -1588,15 +1592,21 @@
// parse out dbus probes by discarding other probe types, store in a
// map
- for (const std::string probe : probeCommand)
+ for (const nlohmann::json& probeJson : probeCommand)
{
+ const std::string* probe = probeJson.get_ptr<const std::string*>();
+ if (probe == nullptr)
+ {
+ std::cerr << "Probe statement wasn't a string, can't parse";
+ continue;
+ }
bool found = false;
boost::container::flat_map<const char*, probe_type_codes,
- cmp_str>::const_iterator probeType;
- for (probeType = PROBE_TYPES.begin();
- probeType != PROBE_TYPES.end(); ++probeType)
+ CmpStr>::const_iterator probeType;
+ for (probeType = probeTypes.begin(); probeType != probeTypes.end();
+ ++probeType)
{
- if (probe.find(probeType->first) != std::string::npos)
+ if (probe->find(probeType->first) != std::string::npos)
{
found = true;
break;
@@ -1607,8 +1617,8 @@
continue;
}
// syntax requires probe before first open brace
- auto findStart = probe.find("(");
- std::string interface = probe.substr(0, findStart);
+ auto findStart = probe->find('(');
+ std::string interface = probe->substr(0, findStart);
dbusProbeInterfaces.emplace(interface);
dbusProbePointers.emplace_back(probePointer);
}
@@ -1619,7 +1629,7 @@
// about a dbus interface
findDbusObjects(std::move(dbusProbePointers),
std::move(dbusProbeInterfaces), shared_from_this());
- if constexpr (DEBUG)
+ if constexpr (debug)
{
std::cerr << __func__ << " " << __LINE__ << "\n";
}
@@ -1636,7 +1646,7 @@
nextScan->dbusProbeObjects = std::move(dbusProbeObjects);
nextScan->run();
- if constexpr (DEBUG)
+ if constexpr (debug)
{
std::cerr << __func__ << " " << __LINE__ << "\n";
}
@@ -1645,7 +1655,7 @@
{
_callback();
- if constexpr (DEBUG)
+ if constexpr (debug)
{
std::cerr << __func__ << " " << __LINE__ << "\n";
}
@@ -1742,7 +1752,7 @@
// we were cancelled
return;
}
- else if (ec)
+ if (ec)
{
std::cerr << "async wait error " << ec << "\n";
return;
@@ -1878,7 +1888,7 @@
};
sdbusplus::bus::match::match match(
- static_cast<sdbusplus::bus::bus&>(*SYSTEM_BUS),
+ static_cast<sdbusplus::bus::bus&>(*systemBus),
"type='signal',member='PropertiesChanged',path='" + path + "'",
eventHandler);
dbusMatches.emplace(path, std::move(match));
@@ -1887,10 +1897,10 @@
int main()
{
// setup connection to dbus
- SYSTEM_BUS = std::make_shared<sdbusplus::asio::connection>(io);
- SYSTEM_BUS->request_name("xyz.openbmc_project.EntityManager");
+ systemBus = std::make_shared<sdbusplus::asio::connection>(io);
+ systemBus->request_name("xyz.openbmc_project.EntityManager");
- sdbusplus::asio::object_server objServer(SYSTEM_BUS);
+ sdbusplus::asio::object_server objServer(systemBus);
std::shared_ptr<sdbusplus::asio::dbus_interface> entityIface =
objServer.add_interface("/xyz/openbmc_project/EntityManager",
@@ -1907,7 +1917,7 @@
// for any reason, expected or otherwise, we'll need a poke to remove
// entities from DBus.
sdbusplus::bus::match::match nameOwnerChangedMatch(
- static_cast<sdbusplus::bus::bus&>(*SYSTEM_BUS),
+ static_cast<sdbusplus::bus::bus&>(*systemBus),
sdbusplus::bus::match::rules::nameOwnerChanged(),
[&](sdbusplus::message::message&) {
propertiesChangedCallback(systemConfiguration, objServer);
@@ -1915,13 +1925,13 @@
// We also need a poke from DBus when new interfaces are created or
// destroyed.
sdbusplus::bus::match::match interfacesAddedMatch(
- static_cast<sdbusplus::bus::bus&>(*SYSTEM_BUS),
+ static_cast<sdbusplus::bus::bus&>(*systemBus),
sdbusplus::bus::match::rules::interfacesAdded(),
[&](sdbusplus::message::message&) {
propertiesChangedCallback(systemConfiguration, objServer);
});
sdbusplus::bus::match::match interfacesRemovedMatch(
- static_cast<sdbusplus::bus::bus&>(*SYSTEM_BUS),
+ static_cast<sdbusplus::bus::bus&>(*systemBus),
sdbusplus::bus::match::rules::interfacesRemoved(),
[&](sdbusplus::message::message&) {
propertiesChangedCallback(systemConfiguration, objServer);
@@ -1974,7 +1984,7 @@
// some boards only show up after power is on, we want to not say they are
// removed until the same state happens
- setupPowerMatch(SYSTEM_BUS);
+ setupPowerMatch(systemBus);
io.run();
diff --git a/src/FruDevice.cpp b/src/FruDevice.cpp
index baab14e..52a83ce 100644
--- a/src/FruDevice.cpp
+++ b/src/FruDevice.cpp
@@ -18,7 +18,6 @@
#include "FruUtils.hpp"
#include "Utils.hpp"
-#include <errno.h>
#include <fcntl.h>
#include <sys/inotify.h>
#include <sys/ioctl.h>
@@ -32,6 +31,7 @@
#include <sdbusplus/asio/object_server.hpp>
#include <array>
+#include <cerrno>
#include <chrono>
#include <ctime>
#include <filesystem>
@@ -57,18 +57,18 @@
}
namespace fs = std::filesystem;
-static constexpr bool DEBUG = false;
-static size_t UNKNOWN_BUS_OBJECT_COUNT = 0;
-constexpr size_t MAX_FRU_SIZE = 512;
-constexpr size_t MAX_EEPROM_PAGE_INDEX = 255;
+static constexpr bool debug = false;
+static size_t unknownBusObjectCount = 0;
+constexpr size_t maxFruSize = 512;
+constexpr size_t maxEepromPageIndex = 255;
constexpr size_t busTimeoutSeconds = 5;
constexpr const char* blacklistPath = PACKAGE_DIR "blacklist.json";
-const static constexpr char* BASEBOARD_FRU_LOCATION =
+const static constexpr char* baseboardFruLocation =
"/etc/fru/baseboard.fru.bin";
-const static constexpr char* I2C_DEV_LOCATION = "/dev";
+const static constexpr char* i2CDevLocation = "/dev";
using DeviceMap = boost::container::flat_map<int, std::vector<uint8_t>>;
using BusMap = boost::container::flat_map<int, std::shared_ptr<DeviceMap>>;
@@ -94,7 +94,7 @@
std::vector<uint8_t>::const_iterator end);
bool updateFRUProperty(
const std::string& assetTag, uint32_t bus, uint32_t address,
- std::string propertyName,
+ const std::string& propertyName,
boost::container::flat_map<
std::pair<size_t, size_t>,
std::shared_ptr<sdbusplus::asio::dbus_interface>>& dbusInterfaceMap);
@@ -138,7 +138,7 @@
static int busStrToInt(const std::string& busName)
{
- auto findBus = busName.rfind("-");
+ auto findBus = busName.rfind('-');
if (findBus == std::string::npos)
{
return -1;
@@ -159,7 +159,7 @@
}
std::string filename = path.filename();
- auto findBus = filename.find("-");
+ auto findBus = filename.find('-');
if (findBus == std::string::npos)
{
return -1;
@@ -222,13 +222,12 @@
// Issue an I2C transaction to first write to_slave_buf_len bytes,then read
// from_slave_buf_len bytes.
-static int i2c_smbus_write_then_read(int file, uint16_t address,
- uint8_t* toSlaveBuf, uint8_t toSlaveBufLen,
- uint8_t* fromSlaveBuf,
- uint8_t fromSlaveBufLen)
+static int i2cSmbusWriteThenRead(int file, uint16_t address,
+ uint8_t* toSlaveBuf, uint8_t toSlaveBufLen,
+ uint8_t* fromSlaveBuf, uint8_t fromSlaveBufLen)
{
- if (toSlaveBuf == NULL || toSlaveBufLen == 0 || fromSlaveBuf == NULL ||
- fromSlaveBufLen == 0)
+ if (toSlaveBuf == nullptr || toSlaveBufLen == 0 ||
+ fromSlaveBuf == nullptr || fromSlaveBufLen == 0)
{
return -1;
}
@@ -264,7 +263,7 @@
}
offset = htobe16(offset);
- return i2c_smbus_write_then_read(
+ return i2cSmbusWriteThenRead(
file, address, reinterpret_cast<uint8_t*>(&offset), 2, buf, len);
}
@@ -290,7 +289,8 @@
return device;
}
-std::set<int> findI2CEeproms(int i2cBus, std::shared_ptr<DeviceMap> devices)
+std::set<int> findI2CEeproms(int i2cBus,
+ const std::shared_ptr<DeviceMap>& devices)
{
std::set<int> foundList;
@@ -399,12 +399,12 @@
continue;
}
// probe
- else if (i2c_smbus_read_byte(file) < 0)
+ if (i2c_smbus_read_byte(file) < 0)
{
continue;
}
- if (DEBUG)
+ if (debug)
{
std::cout << "something at bus " << bus << " addr " << ii
<< "\n";
@@ -529,7 +529,7 @@
return;
}
-static void FindI2CDevices(const std::vector<fs::path>& i2cBuses,
+static void findI2CDevices(const std::vector<fs::path>& i2cBuses,
BusMap& busmap)
{
for (auto& i2cBus : i2cBuses)
@@ -582,7 +582,7 @@
// i2cdetect by default uses the range 0x03 to 0x77, as
// this is what we have tested with, use this range. Could be
// changed in future.
- if (DEBUG)
+ if (debug)
{
std::cerr << "Scanning bus " << bus << "\n";
}
@@ -590,7 +590,7 @@
// fd is closed in this function in case the bus locks up
getBusFRUs(file, 0x03, 0x77, bus, device);
- if (DEBUG)
+ if (debug)
{
std::cerr << "Done scanning bus " << bus << "\n";
}
@@ -613,7 +613,7 @@
}
void run()
{
- FindI2CDevices(_i2cBuses, _busMap);
+ findI2CDevices(_i2cBuses, _busMap);
}
const std::vector<fs::path>& _i2cBuses;
@@ -638,7 +638,7 @@
return ret;
}
-void AddFRUObjectToDbus(
+void addFruObjectToDbus(
std::vector<uint8_t>& device,
boost::container::flat_map<
std::pair<size_t, size_t>,
@@ -653,7 +653,7 @@
<< " address " << address << "\n";
return;
}
- else if (res == resCodes::resWarn)
+ if (res == resCodes::resWarn)
{
std::cerr << "there were warnings while parsing FRU for device at bus "
<< bus << " address " << address << "\n";
@@ -677,8 +677,8 @@
}
else
{
- productName = "UNKNOWN" + std::to_string(UNKNOWN_BUS_OBJECT_COUNT);
- UNKNOWN_BUS_OBJECT_COUNT++;
+ productName = "UNKNOWN" + std::to_string(unknownBusObjectCount);
+ unknownBusObjectCount++;
}
productName = "/xyz/openbmc_project/FruDevice/" + productName;
@@ -706,16 +706,16 @@
// Check if the match named has extra information.
found = true;
- std::smatch base_match;
+ std::smatch baseMatch;
bool match = std::regex_match(
- path, base_match, std::regex(productName + "_(\\d+)$"));
+ path, baseMatch, std::regex(productName + "_(\\d+)$"));
if (match)
{
- if (base_match.size() == 2)
+ if (baseMatch.size() == 2)
{
- std::ssub_match base_sub_match = base_match[1];
- std::string base = base_sub_match.str();
+ std::ssub_match baseSubMatch = baseMatch[1];
+ std::string base = baseSubMatch.str();
int value = std::stoi(base);
highest = (value > highest) ? value : highest;
@@ -741,13 +741,13 @@
{
std::regex_replace(property.second.begin(), property.second.begin(),
- property.second.end(), NON_ASCII_REGEX, "_");
+ property.second.end(), nonAsciiRegex, "_");
if (property.second.empty() && property.first != "PRODUCT_ASSET_TAG")
{
continue;
}
std::string key =
- std::regex_replace(property.first, NON_ASCII_REGEX, "_");
+ std::regex_replace(property.first, nonAsciiRegex, "_");
if (property.first == "PRODUCT_ASSET_TAG")
{
@@ -756,7 +756,7 @@
key, property.second + '\0',
[bus, address, propertyName,
&dbusInterfaceMap](const std::string& req, std::string& resp) {
- if (strcmp(req.c_str(), resp.c_str()))
+ if (strcmp(req.c_str(), resp.c_str()) != 0)
{
// call the method which will update
if (updateFRUProperty(req, bus, address, propertyName,
@@ -777,7 +777,7 @@
{
std::cerr << "illegal key: " << key << "\n";
}
- if (DEBUG)
+ if (debug)
{
std::cout << property.first << ": " << property.second << "\n";
}
@@ -793,7 +793,7 @@
static bool readBaseboardFRU(std::vector<uint8_t>& baseboardFRU)
{
// try to read baseboard fru from file
- std::ifstream baseboardFRUFile(BASEBOARD_FRU_LOCATION, std::ios::binary);
+ std::ifstream baseboardFRUFile(baseboardFruLocation, std::ios::binary);
if (baseboardFRUFile.good())
{
baseboardFRUFile.seekg(0, std::ios_base::end);
@@ -813,7 +813,7 @@
bool writeFRU(uint8_t bus, uint8_t address, const std::vector<uint8_t>& fru)
{
boost::container::flat_map<std::string, std::string> tmp;
- if (fru.size() > MAX_FRU_SIZE)
+ if (fru.size() > maxFruSize)
{
std::cerr << "Invalid fru.size() during writeFRU\n";
return false;
@@ -827,102 +827,98 @@
// baseboard fru
if (bus == 0 && address == 0)
{
- std::ofstream file(BASEBOARD_FRU_LOCATION, std::ios_base::binary);
+ std::ofstream file(baseboardFruLocation, std::ios_base::binary);
if (!file.good())
{
- std::cerr << "Error opening file " << BASEBOARD_FRU_LOCATION
- << "\n";
+ std::cerr << "Error opening file " << baseboardFruLocation << "\n";
throw DBusInternalError();
return false;
}
file.write(reinterpret_cast<const char*>(fru.data()), fru.size());
return file.good();
}
- else
+
+ if (hasEepromFile(bus, address))
{
- if (hasEepromFile(bus, address))
+ auto path = getEepromPath(bus, address);
+ int eeprom = open(path.c_str(), O_RDWR | O_CLOEXEC);
+ if (eeprom < 0)
{
- auto path = getEepromPath(bus, address);
- int eeprom = open(path.c_str(), O_RDWR | O_CLOEXEC);
- if (eeprom < 0)
- {
- std::cerr << "unable to open i2c device " << path << "\n";
- throw DBusInternalError();
- return false;
- }
+ std::cerr << "unable to open i2c device " << path << "\n";
+ throw DBusInternalError();
+ return false;
+ }
- ssize_t writtenBytes = write(eeprom, fru.data(), fru.size());
- if (writtenBytes < 0)
- {
- std::cerr << "unable to write to i2c device " << path << "\n";
- close(eeprom);
- throw DBusInternalError();
- return false;
- }
-
+ ssize_t writtenBytes = write(eeprom, fru.data(), fru.size());
+ if (writtenBytes < 0)
+ {
+ std::cerr << "unable to write to i2c device " << path << "\n";
close(eeprom);
- return true;
- }
-
- std::string i2cBus = "/dev/i2c-" + std::to_string(bus);
-
- int file = open(i2cBus.c_str(), O_RDWR | O_CLOEXEC);
- if (file < 0)
- {
- std::cerr << "unable to open i2c device " << i2cBus << "\n";
- throw DBusInternalError();
- return false;
- }
- if (ioctl(file, I2C_SLAVE_FORCE, address) < 0)
- {
- std::cerr << "unable to set device address\n";
- close(file);
throw DBusInternalError();
return false;
}
- constexpr const size_t RETRY_MAX = 2;
- uint16_t index = 0;
- size_t retries = RETRY_MAX;
- while (index < fru.size())
- {
- if ((index && ((index % (MAX_EEPROM_PAGE_INDEX + 1)) == 0)) &&
- (retries == RETRY_MAX))
- {
- // The 4K EEPROM only uses the A2 and A1 device address bits
- // with the third bit being a memory page address bit.
- if (ioctl(file, I2C_SLAVE_FORCE, ++address) < 0)
- {
- std::cerr << "unable to set device address\n";
- close(file);
- throw DBusInternalError();
- return false;
- }
- }
-
- if (i2c_smbus_write_byte_data(file, static_cast<uint8_t>(index),
- fru[index]) < 0)
- {
- if (!retries--)
- {
- std::cerr << "error writing fru: " << strerror(errno)
- << "\n";
- close(file);
- throw DBusInternalError();
- return false;
- }
- }
- else
- {
- retries = RETRY_MAX;
- index++;
- }
- // most eeproms require 5-10ms between writes
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
- }
- close(file);
+ close(eeprom);
return true;
}
+
+ std::string i2cBus = "/dev/i2c-" + std::to_string(bus);
+
+ int file = open(i2cBus.c_str(), O_RDWR | O_CLOEXEC);
+ if (file < 0)
+ {
+ std::cerr << "unable to open i2c device " << i2cBus << "\n";
+ throw DBusInternalError();
+ return false;
+ }
+ if (ioctl(file, I2C_SLAVE_FORCE, address) < 0)
+ {
+ std::cerr << "unable to set device address\n";
+ close(file);
+ throw DBusInternalError();
+ return false;
+ }
+
+ constexpr const size_t retryMax = 2;
+ uint16_t index = 0;
+ size_t retries = retryMax;
+ while (index < fru.size())
+ {
+ if ((index && ((index % (maxEepromPageIndex + 1)) == 0)) &&
+ (retries == retryMax))
+ {
+ // The 4K EEPROM only uses the A2 and A1 device address bits
+ // with the third bit being a memory page address bit.
+ if (ioctl(file, I2C_SLAVE_FORCE, ++address) < 0)
+ {
+ std::cerr << "unable to set device address\n";
+ close(file);
+ throw DBusInternalError();
+ return false;
+ }
+ }
+
+ if (i2c_smbus_write_byte_data(file, static_cast<uint8_t>(index),
+ fru[index]) < 0)
+ {
+ if (!retries--)
+ {
+ std::cerr << "error writing fru: " << strerror(errno) << "\n";
+ close(file);
+ throw DBusInternalError();
+ return false;
+ }
+ }
+ else
+ {
+ retries = retryMax;
+ index++;
+ }
+ // most eeproms require 5-10ms between writes
+ std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ }
+ close(file);
+ return true;
}
void rescanOneBus(
@@ -972,7 +968,7 @@
}
for (auto& device : *(found->second))
{
- AddFRUObjectToDbus(device.second, dbusInterfaceMap,
+ addFruObjectToDbus(device.second, dbusInterfaceMap,
static_cast<uint32_t>(busNum), device.first);
}
});
@@ -1000,7 +996,7 @@
return;
}
- for (auto busPath : busPaths)
+ for (const auto& busPath : busPaths)
{
i2cBuses.emplace_back(busPath.second);
}
@@ -1020,7 +1016,7 @@
}
dbusInterfaceMap.clear();
- UNKNOWN_BUS_OBJECT_COUNT = 0;
+ unknownBusObjectCount = 0;
// todo, get this from a more sensable place
std::vector<uint8_t> baseboardFRU;
@@ -1035,7 +1031,7 @@
{
for (auto& device : *devicemap.second)
{
- AddFRUObjectToDbus(device.second, dbusInterfaceMap,
+ addFruObjectToDbus(device.second, dbusInterfaceMap,
devicemap.first, device.first);
}
}
@@ -1057,7 +1053,7 @@
bool updateFRUProperty(
const std::string& updatePropertyReq, uint32_t bus, uint32_t address,
- std::string propertyName,
+ const std::string& propertyName,
boost::container::flat_map<
std::pair<size_t, size_t>,
std::shared_ptr<sdbusplus::asio::dbus_interface>>& dbusInterfaceMap)
@@ -1093,34 +1089,33 @@
uint8_t fruAreaOffsetFieldValue = 0;
size_t offset = 0;
- std::string areaName = propertyName.substr(0, propertyName.find("_"));
+ std::string areaName = propertyName.substr(0, propertyName.find('_'));
std::string propertyNamePrefix = areaName + "_";
- auto it = std::find(FRU_AREA_NAMES.begin(), FRU_AREA_NAMES.end(), areaName);
- if (it == FRU_AREA_NAMES.end())
+ auto it = std::find(fruAreaNames.begin(), fruAreaNames.end(), areaName);
+ if (it == fruAreaNames.end())
{
std::cerr << "Can't parse area name for property " << propertyName
<< " \n";
return false;
}
- fruAreas fruAreaToUpdate =
- static_cast<fruAreas>(it - FRU_AREA_NAMES.begin());
+ fruAreas fruAreaToUpdate = static_cast<fruAreas>(it - fruAreaNames.begin());
fruAreaOffsetFieldValue =
fruData[getHeaderAreaFieldOffset(fruAreaToUpdate)];
switch (fruAreaToUpdate)
{
case fruAreas::fruAreaChassis:
offset = 3; // chassis part number offset. Skip fixed first 3 bytes
- fruAreaFieldNames = &CHASSIS_FRU_AREAS;
+ fruAreaFieldNames = &chassisFruAreas;
break;
case fruAreas::fruAreaBoard:
offset = 6; // board manufacturer offset. Skip fixed first 6 bytes
- fruAreaFieldNames = &BOARD_FRU_AREAS;
+ fruAreaFieldNames = &boardFruAreas;
break;
case fruAreas::fruAreaProduct:
// Manufacturer name offset. Skip fixed first 3 product fru bytes
// i.e. version, area length and language code
offset = 3;
- fruAreaFieldNames = &PRODUCT_FRU_AREAS;
+ fruAreaFieldNames = &productFruAreas;
break;
default:
std::cerr << "Don't know how to handle property " << propertyName
@@ -1137,7 +1132,6 @@
size_t fruAreaSize = fruData[fruAreaStart + 1] * fruBlockSize;
size_t fruAreaEnd = fruAreaStart + fruAreaSize;
size_t fruDataIter = fruAreaStart + offset;
- size_t fruUpdateFieldLoc = fruDataIter;
size_t skipToFRUUpdateField = 0;
ssize_t fieldLength;
@@ -1153,7 +1147,7 @@
}
if (!found)
{
- std::size_t pos = propertyName.find(FRU_CUSTOM_FIELD_NAME);
+ std::size_t pos = propertyName.find(fruCustomFieldName);
if (pos == std::string::npos)
{
std::cerr << "PropertyName doesn't exist in FRU Area Vectors: "
@@ -1161,7 +1155,7 @@
return false;
}
std::string fieldNumStr =
- propertyName.substr(pos + FRU_CUSTOM_FIELD_NAME.length());
+ propertyName.substr(pos + fruCustomFieldName.length());
size_t fieldNum = std::stoi(fieldNumStr);
if (fieldNum == 0)
{
@@ -1181,7 +1175,7 @@
}
fruDataIter += 1 + fieldLength;
}
- fruUpdateFieldLoc = fruDataIter;
+ size_t fruUpdateFieldLoc = fruDataIter;
// Push post update fru field bytes to a vector
fieldLength = getFieldLength(fruData[fruUpdateFieldLoc]);
@@ -1405,21 +1399,20 @@
eventHandler);
int fd = inotify_init();
- inotify_add_watch(fd, I2C_DEV_LOCATION,
- IN_CREATE | IN_MOVED_TO | IN_DELETE);
+ inotify_add_watch(fd, i2CDevLocation, IN_CREATE | IN_MOVED_TO | IN_DELETE);
std::array<char, 4096> readBuffer;
std::string pendingBuffer;
// monitor for new i2c devices
boost::asio::posix::stream_descriptor dirWatch(io, fd);
std::function<void(const boost::system::error_code, std::size_t)>
watchI2cBusses = [&](const boost::system::error_code& ec,
- std::size_t bytes_transferred) {
+ std::size_t bytesTransferred) {
if (ec)
{
std::cout << "Callback Error " << ec << "\n";
return;
}
- pendingBuffer += std::string(readBuffer.data(), bytes_transferred);
+ pendingBuffer += std::string(readBuffer.data(), bytesTransferred);
while (pendingBuffer.size() > sizeof(inotify_event))
{
const inotify_event* iEvent =
diff --git a/src/FruUtils.cpp b/src/FruUtils.cpp
index beeb148..945abf4 100644
--- a/src/FruUtils.cpp
+++ b/src/FruUtils.cpp
@@ -32,10 +32,10 @@
#include <linux/i2c.h>
}
-static constexpr bool DEBUG = false;
+static constexpr bool debug = false;
constexpr size_t fruVersion = 1; // Current FRU spec version number is 1
-const std::tm intelEpoch(void)
+std::tm intelEpoch(void)
{
std::tm val = {};
val.tm_year = 1996 - 1900;
@@ -178,10 +178,7 @@
// Return language flag as non english
return false;
}
- else
- {
- return true;
- }
+ return true;
}
/* This function verifies for other offsets to check if they are not
@@ -339,7 +336,7 @@
result["CHASSIS_TYPE"] =
std::to_string(static_cast<int>(*fruBytesIter));
fruBytesIter += 1;
- fruAreaFieldNames = &CHASSIS_FRU_AREAS;
+ fruAreaFieldNames = &chassisFruAreas;
break;
}
case fruAreas::fruAreaBoard:
@@ -370,7 +367,7 @@
result["BOARD_MANUFACTURE_DATE"] = std::string(timeString);
fruBytesIter += 3;
- fruAreaFieldNames = &BOARD_FRU_AREAS;
+ fruAreaFieldNames = &boardFruAreas;
break;
}
case fruAreas::fruAreaProduct:
@@ -380,7 +377,7 @@
std::to_string(static_cast<int>(lang));
isLangEng = checkLangEng(lang);
fruBytesIter += 1;
- fruAreaFieldNames = &PRODUCT_FRU_AREAS;
+ fruAreaFieldNames = &productFruAreas;
break;
}
default:
@@ -408,7 +405,7 @@
{
name =
std::string(getFruAreaName(area)) + "_" +
- FRU_CUSTOM_FIELD_NAME +
+ fruCustomFieldName +
std::to_string(fieldIndex - fruAreaFieldNames->size() + 1);
}
@@ -527,10 +524,7 @@
{
return -1;
}
- else
- {
- return fruFieldTypeLenValue & typeLenMask;
- }
+ return fruFieldTypeLenValue & typeLenMask;
}
bool validateHeader(const std::array<uint8_t, I2C_SMBUS_BLOCK_MAX>& blockData)
@@ -538,7 +532,7 @@
// ipmi spec format version number is currently at 1, verify it
if (blockData[0] != fruVersion)
{
- if (DEBUG)
+ if (debug)
{
std::cerr << "FRU spec version " << (int)(blockData[0])
<< " not supported. Supported version is "
@@ -550,7 +544,7 @@
// verify pad is set to 0
if (blockData[6] != 0x0)
{
- if (DEBUG)
+ if (debug)
{
std::cerr << "PAD value in header is non zero, value is "
<< (int)(blockData[6]) << "\n";
@@ -583,7 +577,7 @@
if (sum != blockData[7])
{
- if (DEBUG)
+ if (debug)
{
std::cerr << "Checksum " << (int)(blockData[7])
<< " is invalid. calculated checksum is " << (int)(sum)
@@ -595,7 +589,7 @@
}
std::vector<uint8_t> readFRUContents(int flag, int file, uint16_t address,
- ReadBlockFunc readBlock,
+ const ReadBlockFunc& readBlock,
const std::string& errorHelp)
{
std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> blockData;
@@ -609,7 +603,7 @@
// check the header checksum
if (!validateHeader(blockData))
{
- if (DEBUG)
+ if (debug)
{
std::cerr << "Illegal header " << errorHelp << "\n";
}
diff --git a/src/Overlay.cpp b/src/Overlay.cpp
index d18cf9d..4454423 100644
--- a/src/Overlay.cpp
+++ b/src/Overlay.cpp
@@ -34,15 +34,14 @@
#include <regex>
#include <string>
-constexpr const char* OUTPUT_DIR = "/tmp/overlays";
-constexpr const char* TEMPLATE_CHAR = "$";
-constexpr const char* HEX_FORMAT_STR = "0x";
-constexpr const char* I2C_DEVS_DIR = "/sys/bus/i2c/devices";
-constexpr const char* MUX_SYMLINK_DIR = "/dev/i2c-mux";
+constexpr const char* outputDir = "/tmp/overlays";
+constexpr const char* templateChar = "$";
+constexpr const char* i2CDevsDir = "/sys/bus/i2c/devices";
+constexpr const char* muxSymlinkDir = "/dev/i2c-mux";
-constexpr const bool DEBUG = false;
+constexpr const bool debug = false;
-std::regex ILLEGAL_NAME_REGEX("[^A-Za-z0-9_]");
+std::regex illegalNameRegex("[^A-Za-z0-9_]");
// helper function to make json types into string
std::string jsonToString(const nlohmann::json& in)
@@ -51,7 +50,7 @@
{
return in.get<std::string>();
}
- else if (in.type() == nlohmann::json::value_t::array)
+ if (in.type() == nlohmann::json::value_t::array)
{
// remove brackets and comma from array
std::string array = in.dump();
@@ -66,17 +65,17 @@
const nlohmann::json::array_t& channelNames)
{
std::error_code ec;
- std::filesystem::path muxSymlinkDir(MUX_SYMLINK_DIR);
- std::filesystem::create_directory(muxSymlinkDir, ec);
+ std::filesystem::path muxSymlinkDirPath(muxSymlinkDir);
+ std::filesystem::create_directory(muxSymlinkDirPath, ec);
// ignore error codes here if the directory already exists
ec.clear();
- std::filesystem::path linkDir = muxSymlinkDir / muxName;
+ std::filesystem::path linkDir = muxSymlinkDirPath / muxName;
std::filesystem::create_directory(linkDir, ec);
std::ostringstream hexAddress;
hexAddress << std::hex << std::setfill('0') << std::setw(4) << address;
- std::filesystem::path devDir(I2C_DEVS_DIR);
+ std::filesystem::path devDir(i2CDevsDir);
devDir /= std::to_string(busIndex) + "-" + hexAddress.str();
for (std::size_t channelIndex = 0; channelIndex < channelNames.size();
@@ -116,7 +115,7 @@
}
static int deleteDevice(const std::string& devicePath,
- std::shared_ptr<uint64_t> address,
+ const std::shared_ptr<uint64_t>& address,
const std::string& destructor)
{
if (!address)
@@ -155,8 +154,8 @@
}
static bool deviceIsCreated(const std::string& devicePath,
- std::shared_ptr<uint64_t> bus,
- std::shared_ptr<uint64_t> address,
+ const std::shared_ptr<uint64_t>& bus,
+ const std::shared_ptr<uint64_t>& address,
const bool retrying)
{
if (!bus || !address)
@@ -184,7 +183,10 @@
}
const std::string directoryName = path->path().filename();
- if (directoryName == busStr + "-" + addressHex)
+ std::string name = busStr;
+ name += "-";
+ name += addressHex;
+ if (directoryName == name)
{
// The first time the BMC boots the kernel has creates a
// filesystem enumerating the I2C devices. The I2C device has not
@@ -217,18 +219,15 @@
}
return true;
}
- else
- {
- path.disable_recursion_pending();
- }
+ path.disable_recursion_pending();
}
return false;
}
static int buildDevice(const std::string& devicePath,
const std::string& parameters,
- std::shared_ptr<uint64_t> bus,
- std::shared_ptr<uint64_t> address,
+ const std::shared_ptr<uint64_t>& bus,
+ const std::shared_ptr<uint64_t>& address,
const std::string& constructor,
const std::string& destructor, const bool createsHWMon,
const size_t retries = 5)
@@ -296,7 +295,7 @@
keyPair.value().type() == nlohmann::json::value_t::string)
{
subsituteString = std::regex_replace(
- keyPair.value().get<std::string>(), ILLEGAL_NAME_REGEX, "_");
+ keyPair.value().get<std::string>(), illegalNameRegex, "_");
name = subsituteString;
}
else
@@ -319,9 +318,9 @@
channels =
keyPair.value().get_ptr<const nlohmann::json::array_t*>();
}
- boost::replace_all(parameters, TEMPLATE_CHAR + keyPair.key(),
+ boost::replace_all(parameters, templateChar + keyPair.key(),
subsituteString);
- boost::replace_all(devicePath, TEMPLATE_CHAR + keyPair.key(),
+ boost::replace_all(devicePath, templateChar + keyPair.key(),
subsituteString);
}
@@ -337,7 +336,7 @@
bool loadOverlays(const nlohmann::json& systemConfiguration)
{
- std::filesystem::create_directory(OUTPUT_DIR);
+ std::filesystem::create_directory(outputDir);
for (auto entity = systemConfiguration.begin();
entity != systemConfiguration.end(); entity++)
{
@@ -374,7 +373,7 @@
// this error message is not printed in all situations.
// If wondering why your device not appearing, add your type to
// the exportTemplates array in the devices.hpp file.
- if constexpr (DEBUG)
+ if constexpr (debug)
{
std::cerr << "Device type " << type
<< " not found in export map whitelist\n";
diff --git a/src/Utils.cpp b/src/Utils.cpp
index a097d06..e92897e 100644
--- a/src/Utils.cpp
+++ b/src/Utils.cpp
@@ -46,7 +46,9 @@
std::vector<fs::path>& foundPaths)
{
if (!fs::exists(dirPath))
+ {
return false;
+ }
std::regex search(matchString);
std::smatch match;
@@ -99,7 +101,7 @@
parser.populateSchema(schemaAdapter, schema);
valijson::Validator validator;
valijson::adapters::NlohmannJsonAdapter targetAdapter(input);
- if (!validator.validate(schema, targetAdapter, NULL))
+ if (!validator.validate(schema, targetAdapter, nullptr))
{
return false;
}
@@ -227,9 +229,9 @@
foundDevicePair.second);
return ret;
}
- else if (nextItemIdx > strPtr->size() ||
- std::find(mathChars.begin(), mathChars.end(),
- strPtr->at(nextItemIdx)) == mathChars.end())
+ if (nextItemIdx > strPtr->size() ||
+ std::find(mathChars.begin(), mathChars.end(),
+ strPtr->at(nextItemIdx)) == mathChars.end())
{
std::string val = std::visit(VariantToStringVisitor(),
foundDevicePair.second);