commit e01d0c36af115ed46d54b5dbbacfe3ad92226bd3
author Ed Tanous <edtanous@google.com> Fri Jun 30 13:21:32 2023 -0700
committer Ed Tanous <ed@tanous.net> Mon Aug 07 17:51:50 2023 +0000
tree 3355d0c47cc9af8068a0a3f7329a8109573f9eb1
parent 1ccf70f116b28a0f78404b914a789ce05f411ddf

Fix bugprone-unchecked-optional-access findings

Clang-tidy has the aforementioned check, which shows a few places in the
core where we ignored the required optional checks.  Fix all uses.
Note, we cannot enable the check that this time because of some weird
code in health.hpp that crashes tidy[1].  That will need to be a future
improvement.

There are tests that call something like
ASSERT(optional)
EXPECT(optional->foo())

While this isn't an actual violation, clang-tidy doesn't seem to be
smart enough to deal with it, so add some explicit checks.

[1] https://github.com/llvm/llvm-project/issues/55530

Tested: Redfish service validator passes.

Change-Id: Ied579cd0b957efc81aff5d5d1091a740a7a2d7e3
Signed-off-by: Ed Tanous <edtanous@google.com>

http/http_client.hpp

diff --git a/http/http_client.hpp b/http/http_client.hpp
index d82c566..71fb885 100644
--- a/http/http_client.hpp
+++ b/http/http_client.hpp
@@ -125,6 +125,7 @@
     {}
 };
 
+namespace http = boost::beast::http;
 class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo>
 {
   private:
@@ -137,10 +138,9 @@
     uint32_t connId;
 
     // Data buffers
-    boost::beast::http::request<boost::beast::http::string_body> req;
-    std::optional<
-        boost::beast::http::response_parser<boost::beast::http::string_body>>
-        parser;
+    http::request<http::string_body> req;
+    using parser_type = http::response_parser<http::string_body>;
+    std::optional<parser_type> parser;
     boost::beast::flat_static_buffer<httpReadBufferSize> buffer;
     Response res;
 
@@ -329,9 +329,10 @@
     {
         state = ConnState::recvInProgress;
 
-        parser.emplace(std::piecewise_construct, std::make_tuple());
+        parser_type& thisParser = parser.emplace(std::piecewise_construct,
+                                                 std::make_tuple());
 
-        parser->body_limit(connPolicy->requestByteLimit);
+        thisParser.body_limit(connPolicy->requestByteLimit);
 
         timer.expires_after(std::chrono::seconds(30));
         timer.async_wait(std::bind_front(onTimeout, weak_from_this()));
@@ -340,14 +341,14 @@
         if (sslConn)
         {
             boost::beast::http::async_read(
-                *sslConn, buffer, *parser,
+                *sslConn, buffer, thisParser,
                 std::bind_front(&ConnectionInfo::afterRead, this,
                                 shared_from_this()));
         }
         else
         {
             boost::beast::http::async_read(
-                conn, buffer, *parser,
+                conn, buffer, thisParser,
                 std::bind_front(&ConnectionInfo::afterRead, this,
                                 shared_from_this()));
         }
@@ -375,6 +376,10 @@
         }
         BMCWEB_LOG_DEBUG("recvMessage() bytes transferred: {}",
                          bytesTransferred);
+        if (!parser)
+        {
+            return;
+        }
         BMCWEB_LOG_DEBUG("recvMessage() data: {}", parser->get().body());
 
         unsigned int respCode = parser->get().result_int();