Add check for globals
We don't follow this cpp core guidelines rule well. This is something
that we should aspire to cleaning up in the future, but for the moment,
lets turn the rule on in clang-tidy to stop the bleeding, add ignores
for the things that we know need some better abstractions, and work on
these over time.
Most of this commit is just adding NOLINTNEXTLINE exceptions for all of
our globals. There was one case in the sensor code where clang
correctly noted that those globals weren't actually const, which got
missed because of the use of auto.
Tested: CI should be good enough for this. Passes clang-tidy.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ieda08fee69a3b209d4b3e9771809a6c41524f066
diff --git a/.clang-tidy b/.clang-tidy
index 7d9979f..bad6111 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -202,6 +202,7 @@
clang-analyzer-webkit.RefCntblBaseVirtualDtor,
cppcoreguidelines-avoid-c-arrays,
cppcoreguidelines-avoid-goto,
+cppcoreguidelines-avoid-non-const-global-variables,
cppcoreguidelines-c-copy-assignment-signature,
cppcoreguidelines-explicit-virtual-functions,
cppcoreguidelines-init-variables,
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 1c1bfd9..45581fd 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -41,6 +41,7 @@
"text/html;charset=UTF-8");
}
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static int connectionCount = 0;
// request body limit size set by the bmcwebHttpReqBodyLimitMb option
diff --git a/include/dbus_monitor.hpp b/include/dbus_monitor.hpp
index 0838f0a..3bad070 100644
--- a/include/dbus_monitor.hpp
+++ b/include/dbus_monitor.hpp
@@ -24,9 +24,11 @@
interfaces;
};
-static boost::container::flat_map<crow::websocket::Connection*,
- DbusWebsocketSession>
- sessions;
+using SessionMap = boost::container::flat_map<crow::websocket::Connection*,
+ DbusWebsocketSession>;
+
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
+static SessionMap sessions;
inline int onPropertyUpdate(sd_bus_message* m, void* userdata,
sd_bus_error* retError)
diff --git a/include/dbus_singleton.hpp b/include/dbus_singleton.hpp
index 2830216..f9b50b0 100644
--- a/include/dbus_singleton.hpp
+++ b/include/dbus_singleton.hpp
@@ -9,6 +9,7 @@
// Initialze before using!
// Please see webserver_main for the example how this variable is initialzed,
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
extern sdbusplus::asio::connection* systemBus;
} // namespace connections
diff --git a/include/forward_unauthorized.hpp b/include/forward_unauthorized.hpp
index 2fc3ee4..850a0b7 100644
--- a/include/forward_unauthorized.hpp
+++ b/include/forward_unauthorized.hpp
@@ -6,6 +6,7 @@
namespace forward_unauthorized
{
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static bool hasWebuiRoute = false;
inline void sendUnauthorized(std::string_view url,
diff --git a/include/hostname_monitor.hpp b/include/hostname_monitor.hpp
index 3fb42e4..cb84ad6 100644
--- a/include/hostname_monitor.hpp
+++ b/include/hostname_monitor.hpp
@@ -11,6 +11,7 @@
{
namespace hostname_monitor
{
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::unique_ptr<sdbusplus::bus::match_t> hostnameSignalMonitor;
inline void installCertificate(const std::filesystem::path& certPath)
diff --git a/include/image_upload.hpp b/include/image_upload.hpp
index 4defbb6..bdb7b95 100644
--- a/include/image_upload.hpp
+++ b/include/image_upload.hpp
@@ -16,6 +16,7 @@
namespace image_upload
{
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::unique_ptr<sdbusplus::bus::match_t> fwUpdateMatcher;
inline void
diff --git a/include/kvm_websocket.hpp b/include/kvm_websocket.hpp
index 79975d2..1a067ac 100644
--- a/include/kvm_websocket.hpp
+++ b/include/kvm_websocket.hpp
@@ -150,9 +150,10 @@
bool doingWrite{false};
};
-static boost::container::flat_map<crow::websocket::Connection*,
- std::unique_ptr<KvmSession>>
- sessions;
+using SessionMap = boost::container::flat_map<crow::websocket::Connection*,
+ std::unique_ptr<KvmSession>>;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
+static SessionMap sessions;
inline void requestRoutes(App& app)
{
diff --git a/include/nbd_proxy.hpp b/include/nbd_proxy.hpp
index ac3a811..68491cb 100644
--- a/include/nbd_proxy.hpp
+++ b/include/nbd_proxy.hpp
@@ -34,7 +34,7 @@
using boost::asio::local::stream_protocol;
static constexpr auto nbdBufferSize = 131088;
-static const char* requiredPrivilegeString = "ConfigureManager";
+constexpr const char* requiredPrivilegeString = "ConfigureManager";
struct NbdProxyServer : std::enable_shared_from_this<NbdProxyServer>
{
@@ -247,9 +247,10 @@
crow::websocket::Connection& connection;
};
-static boost::container::flat_map<crow::websocket::Connection*,
- std::shared_ptr<NbdProxyServer>>
- sessions;
+using SessionMap = boost::container::flat_map<crow::websocket::Connection*,
+ std::shared_ptr<NbdProxyServer>>;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
+static SessionMap sessions;
inline void requestRoutes(App& app)
{
diff --git a/include/obmc_console.hpp b/include/obmc_console.hpp
index d5eaf81..b8f6943 100644
--- a/include/obmc_console.hpp
+++ b/include/obmc_console.hpp
@@ -12,13 +12,18 @@
namespace obmc_console
{
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::unique_ptr<boost::asio::local::stream_protocol::socket> hostSocket;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::array<char, 4096> outputBuffer;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::string inputBuffer;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static boost::container::flat_set<crow::websocket::Connection*> sessions;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static bool doingWrite = false;
inline void doWrite()
diff --git a/include/vm_websocket.hpp b/include/vm_websocket.hpp
index ebf0a69..1d3bf96 100644
--- a/include/vm_websocket.hpp
+++ b/include/vm_websocket.hpp
@@ -14,6 +14,7 @@
namespace obmc_vm
{
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static crow::websocket::Connection* session = nullptr;
// The max network block device buffer size is 128kb plus 16bytes
@@ -157,6 +158,7 @@
inputBuffer;
};
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::shared_ptr<Handler> handler;
inline void requestRoutes(App& app)
diff --git a/include/webroutes.hpp b/include/webroutes.hpp
index 757a272..ec1bb23 100644
--- a/include/webroutes.hpp
+++ b/include/webroutes.hpp
@@ -6,6 +6,7 @@
namespace webroutes
{
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static boost::container::flat_set<std::string> routes;
} // namespace webroutes
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 6f1e91b..d2a4a74 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -77,12 +77,17 @@
} // namespace registries
#ifndef BMCWEB_ENABLE_REDFISH_DBUS_LOG_ENTRIES
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::optional<boost::asio::posix::stream_descriptor> inotifyConn;
static constexpr const char* redfishEventLogDir = "/var/log";
static constexpr const char* redfishEventLogFile = "/var/log/redfish";
static constexpr const size_t iEventSize = sizeof(inotify_event);
+
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static int inotifyFd = -1;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static int dirWatchDesc = -1;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static int fileWatchDesc = -1;
// <ID, timestamp, RedfishLogId, registryPrefix, MessageId, MessageArgs>
diff --git a/redfish-core/lib/certificate_service.hpp b/redfish-core/lib/certificate_service.hpp
index a65f33c..f563379 100644
--- a/redfish-core/lib/certificate_service.hpp
+++ b/redfish-core/lib/certificate_service.hpp
@@ -569,6 +569,7 @@
certFile->getCertFilePath());
}
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::unique_ptr<sdbusplus::bus::match_t> csrMatcher;
/**
* @brief Read data from CSR D-bus object and set to response
diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp
index 00b2ba6..f5c4c3b 100644
--- a/redfish-core/lib/sensors.hpp
+++ b/redfish-core/lib/sensors.hpp
@@ -59,12 +59,12 @@
// clang-format off
namespace dbus
{
-static auto powerPaths = std::to_array<std::string_view>({
+constexpr auto powerPaths = std::to_array<std::string_view>({
"/xyz/openbmc_project/sensors/voltage",
"/xyz/openbmc_project/sensors/power"
});
-static auto sensorPaths = std::to_array<std::string_view>({
+constexpr auto sensorPaths = std::to_array<std::string_view>({
"/xyz/openbmc_project/sensors/power",
"/xyz/openbmc_project/sensors/current",
"/xyz/openbmc_project/sensors/airflow",
@@ -80,7 +80,7 @@
"/xyz/openbmc_project/sensors/utilization"
});
-static auto thermalPaths = std::to_array<std::string_view>({
+constexpr auto thermalPaths = std::to_array<std::string_view>({
"/xyz/openbmc_project/sensors/fan_tach",
"/xyz/openbmc_project/sensors/temperature",
"/xyz/openbmc_project/sensors/fan_pwm"
@@ -89,11 +89,12 @@
} // namespace dbus
// clang-format on
-using sensorPair = std::pair<std::string_view, std::span<std::string_view>>;
+using sensorPair =
+ std::pair<std::string_view, std::span<const std::string_view>>;
static constexpr std::array<sensorPair, 3> paths = {
- {{node::power, std::span<std::string_view>(dbus::powerPaths)},
- {node::sensors, std::span<std::string_view>(dbus::sensorPaths)},
- {node::thermal, std::span<std::string_view>(dbus::thermalPaths)}}};
+ {{node::power, dbus::powerPaths},
+ {node::sensors, dbus::sensorPaths},
+ {node::thermal, dbus::thermalPaths}}};
inline sensor::ReadingType toReadingType(std::string_view sensorType)
{
@@ -203,7 +204,7 @@
SensorsAsyncResp(const std::shared_ptr<bmcweb::AsyncResp>& asyncRespIn,
const std::string& chassisIdIn,
- std::span<std::string_view> typesIn,
+ std::span<const std::string_view> typesIn,
std::string_view subNode) :
asyncResp(asyncRespIn),
chassisId(chassisIdIn), types(typesIn), chassisSubNode(subNode),
@@ -213,7 +214,7 @@
// Store extra data about sensor mapping and return it in callback
SensorsAsyncResp(const std::shared_ptr<bmcweb::AsyncResp>& asyncRespIn,
const std::string& chassisIdIn,
- std::span<std::string_view> typesIn,
+ std::span<const std::string_view> typesIn,
std::string_view subNode,
DataCompleteCb&& creationComplete) :
asyncResp(asyncRespIn),
@@ -225,7 +226,7 @@
// sensor collections expand
SensorsAsyncResp(const std::shared_ptr<bmcweb::AsyncResp>& asyncRespIn,
const std::string& chassisIdIn,
- const std::span<std::string_view> typesIn,
+ std::span<const std::string_view> typesIn,
const std::string_view& subNode, bool efficientExpandIn) :
asyncResp(asyncRespIn),
chassisId(chassisIdIn), types(typesIn), chassisSubNode(subNode),
@@ -288,7 +289,7 @@
const std::shared_ptr<bmcweb::AsyncResp> asyncResp;
const std::string chassisId;
- const std::span<std::string_view> types;
+ const std::span<const std::string_view> types;
const std::string chassisSubNode;
const bool efficientExpand;
@@ -443,7 +444,7 @@
*/
inline void reduceSensorList(
crow::Response& res, std::string_view chassisSubNode,
- std::span<std::string_view> sensorTypes,
+ std::span<const std::string_view> sensorTypes,
const std::vector<std::string>* allSensors,
const std::shared_ptr<std::set<std::string>>& activeSensors)
{
@@ -515,7 +516,8 @@
template <typename Callback>
void getChassis(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
std::string_view chassisId, std::string_view chassisSubNode,
- std::span<std::string_view> sensorTypes, Callback&& callback)
+ std::span<const std::string_view> sensorTypes,
+ Callback&& callback)
{
BMCWEB_LOG_DEBUG << "getChassis enter";
constexpr std::array<std::string_view, 2> interfaces = {
diff --git a/redfish-core/lib/task.hpp b/redfish-core/lib/task.hpp
index fe3b990..6b0096c 100644
--- a/redfish-core/lib/task.hpp
+++ b/redfish-core/lib/task.hpp
@@ -33,6 +33,7 @@
{
constexpr size_t maxTaskCount = 100; // arbitrary limit
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::deque<std::shared_ptr<struct TaskData>> tasks;
constexpr bool completed = true;
diff --git a/redfish-core/lib/update_service.hpp b/redfish-core/lib/update_service.hpp
index faf8948..fe6024b 100644
--- a/redfish-core/lib/update_service.hpp
+++ b/redfish-core/lib/update_service.hpp
@@ -30,11 +30,15 @@
{
// Match signals added on software path
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::unique_ptr<sdbusplus::bus::match_t> fwUpdateMatcher;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::unique_ptr<sdbusplus::bus::match_t> fwUpdateErrorMatcher;
// Only allow one update at a time
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static bool fwUpdateInProgress = false;
// Timer for software available
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
static std::unique_ptr<boost::asio::steady_timer> fwAvailableTimer;
inline static void cleanUp()
diff --git a/src/dbus_singleton.cpp b/src/dbus_singleton.cpp
index f78164f..726c3d4 100644
--- a/src/dbus_singleton.cpp
+++ b/src/dbus_singleton.cpp
@@ -6,7 +6,7 @@
{
namespace connections
{
-
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
sdbusplus::asio::connection* systemBus = nullptr;
} // namespace connections