commit 195d552861198586ad0a7a2e7faf606b9128c411
author Jayanth Othayoth <ojayanth@gmail.com> Sat Jun 07 02:28:07 2025 -0500
committer Jayanth Othayoth <ojayanth@gmail.com> Tue Jun 17 07:03:22 2025 +0000
tree 1374126fd44296cf2ce1707ea1d9654c552fe268
parent 6b580c7ceb0a5073c3ff3d264ab52e2a726dc9d5

clang-tidy: Add bugprone-unchecked-optional-access

Enabled the clang-tidy check `bugprone-unchecked-optional-access` to
improve code safety when working with std::optional.

Fixed the following instances where optional values were accessed
without prior validation.

'''
../libipmid/utils.cpp:173:12: error: unchecked access to optional value [bugprone-unchecked-optional-access,-warnings-as-errors]
  173 |     return cachedService.value();
./chassishandler.cpp:1124:21: error: unchecked access to optional value [bugprone-unchecked-optional-access,-warnings-as-errors]
 1124 |                     sdbusplus::message::convert_from_string<Intrusion::Status>(
'''

Change-Id: I910a6007453be4b01adc9bda36d2d37358de1ace
Signed-off-by: Jayanth Othayoth <ojayanth@gmail.com>

.clang-tidy

diff --git a/.clang-tidy b/.clang-tidy
index 7d68b81..7c0c50d 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -1,4 +1,5 @@
 Checks: '-*,
+    bugprone-unchecked-optional-access,
     modernize-use-nullptr
     '
 

chassishandler.cpp

diff --git a/chassishandler.cpp b/chassishandler.cpp
index 07e5544..f8b548c 100644
--- a/chassishandler.cpp
+++ b/chassishandler.cpp
@@ -1108,32 +1108,38 @@
         {
             ec = ipmi::getDbusProperty<std::string>(
                 ctx, service, path, Intrusion::interface, "Status", propVal);
+
             if (ec)
             {
-                lg2::error("Fail to get Chassis Intrusion Status property "
+                lg2::error("Failed to get Chassis Intrusion Status property "
                            "({SERVICE}/{PATH}/{INTERFACE}): {ERROR}",
                            "SERVICE", service, "PATH", path, "INTERFACE",
                            Intrusion::interface, "ERROR", ec.message());
+                continue;
             }
-            else
+
+            auto statusOpt =
+                sdbusplus::message::convert_from_string<Intrusion::Status>(
+                    propVal);
+            if (statusOpt)
             {
-                // return false if all values are Normal
-                // return true if one value is not Normal
-                // return nullopt when no value can be retrieved from D-Bus
-                auto status =
-                    sdbusplus::message::convert_from_string<Intrusion::Status>(
-                        propVal)
-                        .value();
-                if (status == Intrusion::Status::Normal)
+                if (*statusOpt == Intrusion::Status::Normal)
                 {
                     ret = std::make_optional(false);
                 }
                 else
                 {
                     ret = std::make_optional(true);
-                    return ret;
+                    return ret; // Early return on first non-Normal status
                 }
             }
+            else
+            {
+                lg2::warning(
+                    "Invalid Intrusion::Status value received: {VALUE}",
+                    "VALUE", propVal);
+                return std::nullopt;
+            }
         }
     }
     return ret;

libipmid/utils.cpp

diff --git a/libipmid/utils.cpp b/libipmid/utils.cpp
index f8e8083..7468cf8 100644
--- a/libipmid/utils.cpp
+++ b/libipmid/utils.cpp
@@ -170,6 +170,11 @@
         cachedBusName = bus.get_unique_name();
         cachedService = ::ipmi::getService(bus, intf, path);
     }
+
+    if (!cachedService)
+    {
+        throw std::runtime_error("Service not cached");
+    }
     return cachedService.value();
 }