Turn on a bunch of warnings
Turn on as many warnings as easily possible from:
https://github.com/lefticus/cppbestpractices/blob/e73393f25a85f83fed7399d8b65cb117d00b2231/02-Use_the_Tools_Available.md#L100
Tested:
ipmitool sensor list still works
Change-Id: Ied8fa66de9fcd25e448f8048c4f8216b426b6f55
Signed-off-by: James Feist <james.feist@linux.intel.com>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index af1a41c..564fce2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -3,7 +3,33 @@
include (ExternalProject)
set (CMAKE_CXX_STANDARD 17)
set (CMAKE_CXX_STANDARD_REQUIRED ON)
-set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -lstdc++fs -Werror")
+set (
+ CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -lstdc++fs \
+ -Werror \
+ -Wall \
+ -Wextra \
+ -Wnon-virtual-dtor \
+ -Wold-style-cast \
+ -Wcast-align \
+ -Wunused \
+ -Woverloaded-virtual \
+ -Wpedantic \
+ -Wmisleading-indentation \
+ -Wduplicated-cond \
+ -Wduplicated-branches \
+ -Wlogical-op \
+ -Wnull-dereference \
+ -Wuseless-cast \
+ -Wdouble-promotion \
+ -Wformat=2 \
+ -Wno-sign-compare \
+ -Wno-reorder \
+"
+)
+# todo: get rid of nos, add the below:
+# -Wshadow \
+# -Wconversion \
set (CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH})
@@ -53,7 +79,7 @@
if (NOT YOCTO)
option (ENABLE_TEST "Enable Google Test" OFF)
- include_directories (${CMAKE_CURRENT_SOURCE_DIR}/include/non-yocto)
+ include_directories (SYSTEM ${CMAKE_CURRENT_SOURCE_DIR}/include/non-yocto)
externalproject_add (
Boost URL
@@ -63,7 +89,7 @@
"${CMAKE_BINARY_DIR}/boost-build" CONFIGURE_COMMAND "" BUILD_COMMAND ""
INSTALL_COMMAND ""
)
- include_directories (${CMAKE_BINARY_DIR}/boost-src)
+ include_directories (SYSTEM ${CMAKE_BINARY_DIR}/boost-src)
set (CMAKE_PREFIX_PATH ${CMAKE_BINARY_DIR}/boost-src ${CMAKE_PREFIX_PATH})
# requires apt install autoconf-archive and autoconf
@@ -76,7 +102,7 @@
&& ./bootstrap.sh && ./configure --enable-transaction
&& make -j libsdbusplus.la INSTALL_COMMAND ""
LOG_DOWNLOAD ON)
- include_directories (${CMAKE_BINARY_DIR}/sdbusplus-src)
+ include_directories (SYSTEM ${CMAKE_BINARY_DIR}/sdbusplus-src)
link_directories (${CMAKE_BINARY_DIR}/sdbusplus-src/.libs)
externalproject_add (nlohmann-json PREFIX
@@ -86,7 +112,7 @@
BINARY_DIR ${CMAKE_BINARY_DIR}/nlohmann-json-build
CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND
"" LOG_DOWNLOAD ON)
- include_directories (${CMAKE_BINARY_DIR}/nlohmann-json-src/include)
+ include_directories (SYSTEM ${CMAKE_BINARY_DIR}/nlohmann-json-src/include)
if (ENABLE_TEST)
option (HUNTER_ENABLED "Enable hunter package pulling" ON)
hunter_add_package (GTest)
@@ -234,9 +260,10 @@
if (NOT DISABLE_MCUTEMP)
install (TARGETS mcutempsensor DESTINATION bin)
- install (FILES
- ${SERVICE_FILE_SRC_DIR}/xyz.openbmc_project.mcutempsensor.service
- DESTINATION ${SERVICE_FILE_INSTALL_DIR})
+ install (
+ FILES ${SERVICE_FILE_SRC_DIR}/xyz.openbmc_project.mcutempsensor.service
+ DESTINATION ${SERVICE_FILE_INSTALL_DIR}
+ )
endif ()
if (NOT DISABLE_PSU)
diff --git a/include/CPUSensor.hpp b/include/CPUSensor.hpp
index 1c25655..5f48674 100644
--- a/include/CPUSensor.hpp
+++ b/include/CPUSensor.hpp
@@ -94,7 +94,7 @@
bool resp = true;
try
{
- line.request({"adcsensor", gpiod::line_request::DIRECTION_INPUT});
+ line.request({"adcsensor", gpiod::line_request::DIRECTION_INPUT, 0});
resp = !line.get_value();
}
catch (std::system_error&)
diff --git a/src/ADCSensor.cpp b/src/ADCSensor.cpp
index c94f251..2141ac4 100644
--- a/src/ADCSensor.cpp
+++ b/src/ADCSensor.cpp
@@ -165,7 +165,7 @@
}
errCount = 0;
}
- catch (std::invalid_argument)
+ catch (std::invalid_argument&)
{
errCount++;
}
diff --git a/src/ADCSensorMain.cpp b/src/ADCSensorMain.cpp
index 7cb31d0..188aad9 100644
--- a/src/ADCSensorMain.cpp
+++ b/src/ADCSensorMain.cpp
@@ -248,7 +248,7 @@
}
}
-int main(int argc, char** argv)
+int main()
{
boost::asio::io_service io;
auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
diff --git a/src/CPUSensorMain.cpp b/src/CPUSensorMain.cpp
index 2f2319d..2588221 100644
--- a/src/CPUSensorMain.cpp
+++ b/src/CPUSensorMain.cpp
@@ -154,7 +154,7 @@
bus = std::stoi(busStr);
addr = std::stoi(addrStr, 0, 16);
}
- catch (std::invalid_argument)
+ catch (std::invalid_argument&)
{
continue;
}
@@ -243,7 +243,7 @@
auto directory = hwmonNamePath.parent_path();
std::vector<fs::path> inputPaths;
- if (!findFiles(fs::path(directory), R"(temp\d+_input$)", inputPaths, 0))
+ if (!findFiles(directory, R"(temp\d+_input$)", inputPaths, 0))
{
std::cerr << "No temperature sensors in system\n";
continue;
@@ -645,7 +645,7 @@
return false;
}
-int main(int argc, char** argv)
+int main()
{
boost::asio::io_service io;
auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
diff --git a/src/ChassisIntrusionSensor.cpp b/src/ChassisIntrusionSensor.cpp
index 4bf7fdc..f10e9b9 100644
--- a/src/ChassisIntrusionSensor.cpp
+++ b/src/ChassisIntrusionSensor.cpp
@@ -43,9 +43,6 @@
// Status bit field masks
const static constexpr size_t pchRegMaskIntrusion = 0x01;
-// Hold current PCH register values
-static unsigned int intrudeValue;
-
// gpio sysfs path
constexpr const char* gpioPath = "/sys/class/gpio/";
@@ -104,7 +101,7 @@
return -1;
}
- unsigned int statusValue;
+ int statusValue;
unsigned int statusMask = pchRegMaskIntrusion;
unsigned int statusReg = pchStatusRegIntrusion;
diff --git a/src/ExitAirTempSensor.cpp b/src/ExitAirTempSensor.cpp
index a4aba4b..972b42b 100644
--- a/src/ExitAirTempSensor.cpp
+++ b/src/ExitAirTempSensor.cpp
@@ -266,8 +266,7 @@
void CFMSensor::createMaxCFMIface(void)
{
- cfmLimitIface->register_property("Limit", static_cast<double>(c2) * maxCFM *
- tachs.size());
+ cfmLimitIface->register_property("Limit", c2 * maxCFM * tachs.size());
cfmLimitIface->initialize();
}
@@ -686,7 +685,7 @@
(qMax - qMin) * (cfm - qMin));
}
- totalPower *= powerFactor;
+ totalPower *= static_cast<double>(powerFactor);
totalPower += pOffset;
if (totalPower == 0)
@@ -709,7 +708,7 @@
// Calculate the exit air temp
// Texit = Tfp + (1.76 * TotalPower / CFM * Faltitude)
- double reading = 1.76 * totalPower * altitudeFactor;
+ double reading = 1.76 * totalPower * static_cast<double>(altitudeFactor);
reading /= cfm;
reading += inletTemp;
@@ -890,7 +889,7 @@
"GetManagedObjects");
}
-int main(int argc, char** argv)
+int main()
{
boost::asio::io_service io;
@@ -906,7 +905,7 @@
boost::asio::deadline_timer configTimer(io);
std::function<void(sdbusplus::message::message&)> eventHandler =
- [&](sdbusplus::message::message& message) {
+ [&](sdbusplus::message::message&) {
configTimer.expires_from_now(boost::posix_time::seconds(1));
// create a timer because normally multiple properties change
configTimer.async_wait([&](const boost::system::error_code& ec) {
diff --git a/src/FanMain.cpp b/src/FanMain.cpp
index eeface8..b0237c2 100644
--- a/src/FanMain.cpp
+++ b/src/FanMain.cpp
@@ -165,7 +165,8 @@
sensorData = &(sensor.second);
break;
}
- else if (baseType == "xyz.openbmc_project.Configuration.I2CFan")
+ else if (baseType ==
+ std::string("xyz.openbmc_project.Configuration.I2CFan"))
{
auto findBus = baseConfiguration->second.find("Bus");
auto findAddress = baseConfiguration->second.find("Address");
@@ -374,7 +375,7 @@
"org.freedesktop.DBus.ObjectManager", "GetManagedObjects");
}
-int main(int argc, char** argv)
+int main()
{
boost::asio::io_service io;
auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
@@ -435,7 +436,7 @@
// redundancy sensor
std::function<void(sdbusplus::message::message&)> redundancyHandler =
[&tachSensors, &systemBus,
- &objectServer](sdbusplus::message::message& message) {
+ &objectServer](sdbusplus::message::message&) {
createRedundancySensor(tachSensors, systemBus, objectServer);
};
auto match = std::make_unique<sdbusplus::bus::match::match>(
@@ -443,7 +444,7 @@
"type='signal',member='PropertiesChanged',path_namespace='" +
std::string(inventoryPath) + "',arg0namespace='" +
redundancyConfiguration + "'",
- redundancyHandler);
+ std::move(redundancyHandler));
matches.emplace_back(std::move(match));
io.run();
diff --git a/src/HwmonTempMain.cpp b/src/HwmonTempMain.cpp
index 2b1d7c1..7eaea30 100644
--- a/src/HwmonTempMain.cpp
+++ b/src/HwmonTempMain.cpp
@@ -79,7 +79,7 @@
continue; // already searched this path
}
- auto device = fs::path(directory / "device");
+ fs::path device = directory / "device";
std::string deviceName = fs::canonical(device).stem();
auto findHyphen = deviceName.find("-");
if (findHyphen == std::string::npos)
@@ -97,7 +97,7 @@
bus = std::stoi(busStr);
addr = std::stoi(addrStr, 0, 16);
}
- catch (std::invalid_argument)
+ catch (std::invalid_argument&)
{
continue;
}
@@ -208,7 +208,7 @@
}
}
-int main(int argc, char** argv)
+int main()
{
boost::asio::io_service io;
auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
diff --git a/src/HwmonTempSensor.cpp b/src/HwmonTempSensor.cpp
index 09161e4..16742ce 100644
--- a/src/HwmonTempSensor.cpp
+++ b/src/HwmonTempSensor.cpp
@@ -103,7 +103,7 @@
{
float nvalue = std::stof(response);
nvalue /= sensorScaleFactor;
- if (nvalue != value)
+ if (static_cast<double>(nvalue) != value)
{
updateValue(nvalue);
}
diff --git a/src/IpmbSensor.cpp b/src/IpmbSensor.cpp
index 738ef57..b3cc104 100644
--- a/src/IpmbSensor.cpp
+++ b/src/IpmbSensor.cpp
@@ -422,7 +422,7 @@
}
}
-int main(int argc, char** argv)
+int main()
{
boost::asio::io_service io;
@@ -437,7 +437,7 @@
boost::asio::deadline_timer configTimer(io);
std::function<void(sdbusplus::message::message&)> eventHandler =
- [&](sdbusplus::message::message& message) {
+ [&](sdbusplus::message::message&) {
configTimer.expires_from_now(boost::posix_time::seconds(1));
// create a timer because normally multiple properties change
configTimer.async_wait([&](const boost::system::error_code& ec) {
diff --git a/src/MCUTempSensor.cpp b/src/MCUTempSensor.cpp
index fd45327..0b621d0 100644
--- a/src/MCUTempSensor.cpp
+++ b/src/MCUTempSensor.cpp
@@ -105,7 +105,6 @@
{
std::string i2cBus = "/dev/i2c-" + std::to_string(busId);
int fd = open(i2cBus.c_str(), O_RDWR);
- size_t i = 0;
if (fd < 0)
{
@@ -172,7 +171,7 @@
double v = static_cast<double>(temp) / 1000;
if constexpr (debug)
{
- std::cerr << "Value update to " << (double)v << "raw reading "
+ std::cerr << "Value update to " << v << "raw reading "
<< static_cast<int>(temp) << "\n";
}
updateValue(v);
@@ -264,7 +263,7 @@
"GetManagedObjects");
}
-int main(int argc, char** argv)
+int main()
{
boost::asio::io_service io;
auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
@@ -276,7 +275,7 @@
boost::asio::deadline_timer configTimer(io);
std::function<void(sdbusplus::message::message&)> eventHandler =
- [&](sdbusplus::message::message& message) {
+ [&](sdbusplus::message::message&) {
configTimer.expires_from_now(boost::posix_time::seconds(1));
// create a timer because normally multiple properties change
configTimer.async_wait([&](const boost::system::error_code& ec) {
diff --git a/src/PSUEvent.cpp b/src/PSUEvent.cpp
index 5bfa1e4..84b7097 100644
--- a/src/PSUEvent.cpp
+++ b/src/PSUEvent.cpp
@@ -31,7 +31,7 @@
"/xyz/openbmc_project/State/Decorator/" + psuName + "_" +
combineEventName,
"xyz.openbmc_project.State.Decorator.OperationalStatus");
- eventInterface->register_property("functional", bool(true));
+ eventInterface->register_property("functional", true);
if (!eventInterface->initialize())
{
diff --git a/src/PSUSensor.cpp b/src/PSUSensor.cpp
index fdd453a..58dee54 100644
--- a/src/PSUSensor.cpp
+++ b/src/PSUSensor.cpp
@@ -97,7 +97,7 @@
float nvalue = std::stof(response);
responseStream.clear();
nvalue /= sensorFactor;
- if (nvalue != value)
+ if (static_cast<double>(nvalue) != value)
{
updateValue(nvalue);
}
diff --git a/src/PSUSensorMain.cpp b/src/PSUSensorMain.cpp
index 99f6701..49c3a08 100644
--- a/src/PSUSensorMain.cpp
+++ b/src/PSUSensorMain.cpp
@@ -198,7 +198,7 @@
continue; // check if path has already been searched
}
- auto device = fs::path(directory / "device");
+ fs::path device = directory / "device";
std::string deviceName = fs::canonical(device).stem();
auto findHyphen = deviceName.find("-");
if (findHyphen == std::string::npos)
@@ -217,7 +217,7 @@
bus = std::stoi(busStr);
addr = std::stoi(addrStr, 0, 16);
}
- catch (std::invalid_argument)
+ catch (std::invalid_argument&)
{
continue;
}
@@ -309,7 +309,7 @@
} while (findPSUName != baseConfig->second.end());
std::vector<fs::path> sensorPaths;
- if (!findFiles(fs::path(directory), R"(\w\d+_input$)", sensorPaths, 0))
+ if (!findFiles(directory, R"(\w\d+_input$)", sensorPaths, 0))
{
std::cerr << "No PSU non-label sensor in PSU\n";
continue;
@@ -336,7 +336,7 @@
std::string labelPathStr =
boost::replace_all_copy(sensorNameStr, "input", "label");
std::vector<fs::path> labelPaths;
- if (!findFiles(fs::path(directory), labelPathStr, labelPaths, 0))
+ if (!findFiles(directory, labelPathStr, labelPaths, 0))
{
std::cerr << "No PSU non-label sensor in PSU\n";
continue;
@@ -482,7 +482,7 @@
{"FanFault", {"fan1_alarm", "fan2_alarm", "fan1_fault", "fan2_fault"}}};
}
-int main(int argc, char** argv)
+int main()
{
boost::asio::io_service io;
auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
diff --git a/src/PwmSensor.cpp b/src/PwmSensor.cpp
index 6402ad0..d8a7a17 100644
--- a/src/PwmSensor.cpp
+++ b/src/PwmSensor.cpp
@@ -22,7 +22,6 @@
#include <sdbusplus/asio/object_server.hpp>
static constexpr size_t pwmMax = 255;
-static constexpr size_t pwmMin = 0;
PwmSensor::PwmSensor(const std::string& name, const std::string& sysPath,
sdbusplus::asio::object_server& objectServer,
@@ -36,7 +35,7 @@
"/xyz/openbmc_project/sensors/fan_pwm/" + name,
"xyz.openbmc_project.Sensor.Value");
uint32_t pwmValue = getValue(false);
- double fValue = 100.0 * (static_cast<float>(pwmValue) / pwmMax);
+ double fValue = 100.0 * (static_cast<double>(pwmValue) / pwmMax);
sensorInterface->register_property(
"Value", fValue,
[this](const double& req, double& resp) {
@@ -58,7 +57,7 @@
return 1;
},
[this](double& curVal) {
- float value = 100.0 * (static_cast<float>(getValue()) / pwmMax);
+ double value = 100.0 * (static_cast<double>(getValue()) / pwmMax);
if (curVal != value)
{
curVal = value;
@@ -78,7 +77,7 @@
controlInterface->register_property(
"Target", static_cast<uint64_t>(pwmValue),
[this](const uint64_t& req, uint64_t& resp) {
- if (req > pwmMax || req < pwmMin)
+ if (req > pwmMax)
{
throw std::runtime_error("Value out of range");
return -1;
@@ -149,7 +148,7 @@
uint32_t value = std::stoi(line);
return value;
}
- catch (std::invalid_argument)
+ catch (std::invalid_argument&)
{
std::cerr << "Error reading pwm at " << sysPath << "\n";
// throw if not initial read to be caught by dbus bindings
diff --git a/src/TachSensor.cpp b/src/TachSensor.cpp
index 45ac7ac..46f5389 100644
--- a/src/TachSensor.cpp
+++ b/src/TachSensor.cpp
@@ -142,7 +142,7 @@
std::getline(responseStream, response);
float nvalue = std::stof(response);
responseStream.clear();
- if (nvalue != value)
+ if (static_cast<double>(nvalue) != value)
{
updateValue(nvalue);
}
diff --git a/src/Thresholds.cpp b/src/Thresholds.cpp
index 3dd444d..f07f086 100644
--- a/src/Thresholds.cpp
+++ b/src/Thresholds.cpp
@@ -333,7 +333,7 @@
const std::string& inputPath, const double& scaleFactor,
const double& offset)
{
- for (auto& type : attrTypes)
+ for (const std::string& type : attrTypes)
{
auto attrPath = boost::replace_all_copy(inputPath, "input", type);
std::ifstream attrFile(attrPath);