Check optionals in tidy
clang-tidy-18 makes this feature stable enough for us to use in general.
Enable the check, and fix the couple of regressions that have snuck in
since we last ran the check.
Tidy seems to not be able to understand that ASSERT will not continue,
so if we ASSERT a std::optional, it's not a bug. Add explicit checks to
keep tidy happy.
Tested: clang-tidy passes.
Change-Id: I0986453851da5471056a7b47b8ad57a9801df259
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/.clang-tidy b/.clang-tidy
index c7963c0..4862d2c 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -58,6 +58,7 @@
bugprone-terminating-continue,
bugprone-throw-keyword-missing,
bugprone-too-small-loop-variable,
+bugprone-unchecked-optional-access,
bugprone-undefined-memory-manipulation,
bugprone-undelegated-constructor,
bugprone-unhandled-exception-at-new,
diff --git a/http/http2_connection.hpp b/http/http2_connection.hpp
index 749ac28..4b1b2dc 100644
--- a/http/http2_connection.hpp
+++ b/http/http2_connection.hpp
@@ -104,7 +104,10 @@
}
Http2StreamData& stream = streamIt->second;
BMCWEB_LOG_DEBUG("File read callback length: {}", length);
-
+ if (!stream.writer)
+ {
+ return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+ }
boost::beast::error_code ec;
boost::optional<std::pair<boost::asio::const_buffer, bool>> out =
stream.writer->getWithMaxSize(ec, length);
@@ -230,10 +233,11 @@
close();
return -1;
}
- if (it->second.reqReader)
+ auto& reqReader = it->second.reqReader;
+ if (reqReader)
{
boost::beast::error_code ec;
- it->second.reqReader->finish(ec);
+ reqReader->finish(ec);
if (ec)
{
BMCWEB_LOG_CRITICAL("Failed to finalize payload");
@@ -289,15 +293,17 @@
close();
return -1;
}
- if (!thisStream->second.reqReader)
+
+ std::optional<bmcweb::HttpBody::reader>& reqReader =
+ thisStream->second.reqReader;
+ if (!reqReader)
{
- thisStream->second.reqReader.emplace(
- thisStream->second.req.req.base(),
- thisStream->second.req.req.body());
+ BMCWEB_LOG_ERROR("No reader init {}", streamId);
+ close();
+ return -1;
}
boost::beast::error_code ec;
- thisStream->second.reqReader->put(boost::asio::const_buffer(data, len),
- ec);
+ reqReader->put(boost::asio::const_buffer(data, len), ec);
if (ec)
{
BMCWEB_LOG_CRITICAL("Failed to write payload");
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index 8bd4cdc..8ef8b5a 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -1775,54 +1775,43 @@
return;
}
- const std::string* addr = nullptr;
- uint8_t prefix = 0;
-
// Find the address and prefixLength values. Any values that are
// not explicitly provided are assumed to be unmodified from the
// current state of the interface. Merge existing state into the
// current request.
- if (address)
+ if (!address)
{
- addr = &(*address);
- }
- else if (nicIpEntry != ipv6Data.end())
- {
- addr = &(nicIpEntry->address);
- }
- else
- {
- messages::propertyMissing(asyncResp->res,
- pathString + "/Address");
- return;
+ if (nicIpEntry == ipv6Data.end())
+ {
+ messages::propertyMissing(asyncResp->res,
+ pathString + "/Address");
+ return;
+ }
+ address = nicIpEntry->address;
}
- if (prefixLength)
+ if (!prefixLength)
{
- prefix = *prefixLength;
- }
- else if (nicIpEntry != ipv6Data.end())
- {
- prefix = nicIpEntry->prefixLength;
- }
- else
- {
- messages::propertyMissing(asyncResp->res,
- pathString + "/PrefixLength");
- return;
+ if (nicIpEntry == ipv6Data.end())
+ {
+ messages::propertyMissing(asyncResp->res,
+ pathString + "/PrefixLength");
+ return;
+ }
+ prefixLength = nicIpEntry->prefixLength;
}
if (nicIpEntry != ipv6Data.end())
{
deleteAndCreateIPAddress(IpVersion::IpV6, ifaceId,
- nicIpEntry->id, prefix, *addr, "",
- asyncResp);
+ nicIpEntry->id, *prefixLength,
+ *address, "", asyncResp);
nicIpEntry = getNextStaticIpEntry(++nicIpEntry,
ipv6Data.cend());
}
else
{
- createIPv6(ifaceId, *prefixLength, *addr, asyncResp);
+ createIPv6(ifaceId, *prefixLength, *address, asyncResp);
}
entryIdx++;
}
diff --git a/redfish-core/lib/fan.hpp b/redfish-core/lib/fan.hpp
index 8915229..39b9e2a 100644
--- a/redfish-core/lib/fan.hpp
+++ b/redfish-core/lib/fan.hpp
@@ -50,11 +50,11 @@
inline void getFanPaths(
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const std::optional<std::string>& validChassisPath,
+ const std::string& validChassisPath,
const std::function<void(const dbus::utility::MapperGetSubTreePathsResponse&
fanPaths)>& callback)
{
- sdbusplus::message::object_path endpointPath{*validChassisPath};
+ sdbusplus::message::object_path endpointPath{validChassisPath};
endpointPath /= "cooled_by";
dbus::utility::getAssociatedSubTreePaths(
@@ -101,7 +101,7 @@
asyncResp->res.jsonValue["Members"] = nlohmann::json::array();
asyncResp->res.jsonValue["Members@odata.count"] = 0;
- getFanPaths(asyncResp, validChassisPath,
+ getFanPaths(asyncResp, *validChassisPath,
std::bind_front(updateFanList, asyncResp, chassisId));
}
diff --git a/test/redfish-core/lib/update_service_test.cpp b/test/redfish-core/lib/update_service_test.cpp
index 01f2a0e..6b90aef 100644
--- a/test/redfish-core/lib/update_service_test.cpp
+++ b/test/redfish-core/lib/update_service_test.cpp
@@ -15,14 +15,22 @@
// No protocol, schema on url
std::optional<TftpUrl> ret = parseTftpUrl("tftp://1.1.1.1/path",
std::nullopt, res);
- ASSERT_NE(ret, std::nullopt);
+ ASSERT_TRUE(ret);
+ if (!ret)
+ {
+ return;
+ }
EXPECT_EQ(ret->tftpServer, "1.1.1.1");
EXPECT_EQ(ret->fwFile, "path");
}
{
// Protocol, no schema on url
std::optional<TftpUrl> ret = parseTftpUrl("1.1.1.1/path", "TFTP", res);
- ASSERT_NE(ret, std::nullopt);
+ ASSERT_TRUE(ret);
+ if (!ret)
+ {
+ return;
+ }
EXPECT_EQ(ret->tftpServer, "1.1.1.1");
EXPECT_EQ(ret->fwFile, "path");
}
@@ -30,7 +38,11 @@
// Both protocl and schema on url
std::optional<TftpUrl> ret = parseTftpUrl("tftp://1.1.1.1/path", "TFTP",
res);
- ASSERT_NE(ret, std::nullopt);
+ ASSERT_TRUE(ret);
+ if (!ret)
+ {
+ return;
+ }
EXPECT_EQ(ret->tftpServer, "1.1.1.1");
EXPECT_EQ(ret->fwFile, "path");
}