From 016842f0941045610aaf87f5c7775d1386e4b87e Mon Sep 17 00:00:00 2001 From: Sumitkumar Satpute Date: Tue, 30 Jun 2026 09:04:47 +0200 Subject: [PATCH 1/3] Improve nodiscard adoption for OSAL call sites Signed-off-by: Sumitkumar Satpute --- .../control_client/src/details/control_client_impl.cpp | 8 ++++---- .../details/ifappl/MonitorIfDaemon_UT.cpp | 2 +- .../src/daemon/src/osal/security_policy.hpp | 2 +- score/launch_manager/src/daemon/src/osal/semaphore.hpp | 10 +++++----- .../src/daemon/src/osal/set_affinity.hpp | 2 +- .../launch_manager/src/daemon/src/osal/set_groups.hpp | 2 +- .../details/process_info_node.cpp | 6 +++--- .../src/details/report_running_impl.cpp | 7 ++++++- 8 files changed, 22 insertions(+), 17 deletions(-) diff --git a/score/launch_manager/src/control_client/src/details/control_client_impl.cpp b/score/launch_manager/src/control_client/src/details/control_client_impl.cpp index 185b072d9..6ab243ddc 100644 --- a/score/launch_manager/src/control_client/src/details/control_client_impl.cpp +++ b/score/launch_manager/src/control_client/src/details/control_client_impl.cpp @@ -94,7 +94,7 @@ ControlClientImpl::ControlClientImpl(std::function(ipc_request_semaphore_.init(1U, false)); ipc_response_thread_ = std::make_unique(&ControlClientImpl::run, this); } @@ -107,7 +107,7 @@ ControlClientImpl::~ControlClientImpl() noexcept { ipc_response_thread_->join(); } - ipc_request_semaphore_.deinit(); + static_cast(ipc_request_semaphore_.deinit()); } void ControlClientImpl::run() { @@ -284,7 +284,7 @@ score::concurrency::InterruptibleFuture ControlClientImpl::SendIpcMessage( } // we definitely shouldn't forget to release semaphore - ipc_request_semaphore_.post(); + static_cast(ipc_request_semaphore_.post()); } else { @@ -375,7 +375,7 @@ score::Result ControlClientImpl::GetExecutionEr } // we definitely shouldn't forget to release semaphore - ipc_request_semaphore_.post(); + static_cast(ipc_request_semaphore_.post()); } // else not needed as kCommunicationError is the default return value } diff --git a/score/launch_manager/src/daemon/src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp b/score/launch_manager/src/daemon/src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp index 554bcbc5b..9fd808f6e 100644 --- a/score/launch_manager/src/daemon/src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp +++ b/score/launch_manager/src/daemon/src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp @@ -74,7 +74,7 @@ struct MonitorIfDaemonFixture /// Initialize the IPC server so that peek/pop/hasOverflow use real shared memory. void initIpc() { - ipcServer.init(makeUniqueIpcName()); + static_cast(ipcServer.init(makeUniqueIpcName())); } /// Drive the process to the 'running' state and notify observers. diff --git a/score/launch_manager/src/daemon/src/osal/security_policy.hpp b/score/launch_manager/src/daemon/src/osal/security_policy.hpp index 1298eb33c..81ee606ad 100644 --- a/score/launch_manager/src/daemon/src/osal/security_policy.hpp +++ b/score/launch_manager/src/daemon/src/osal/security_policy.hpp @@ -19,7 +19,7 @@ namespace internal { namespace osal { -int setSecurityPolicy(const char* policy); +[[nodiscard]] int setSecurityPolicy(const char* policy); } diff --git a/score/launch_manager/src/daemon/src/osal/semaphore.hpp b/score/launch_manager/src/daemon/src/osal/semaphore.hpp index 213127a7e..edf4084c0 100644 --- a/score/launch_manager/src/daemon/src/osal/semaphore.hpp +++ b/score/launch_manager/src/daemon/src/osal/semaphore.hpp @@ -63,7 +63,7 @@ class Semaphore final { /// @return An OsalReturnType indicating the result of the operation. /// - `OsalReturnType::KSuccess`: The semaphore was successfully initialized. /// - `OsalReturnType::KFail`: An error occurred during the initialization. - OsalReturnType init(uint32_t value, bool shared); + [[nodiscard]] OsalReturnType init(uint32_t value, bool shared); /// @brief Destroys the semaphore and releases any resources associated with it. /// This method uses `sem_destroy` to destroy the semaphore: @@ -72,7 +72,7 @@ class Semaphore final { /// @return An OsalReturnType indicating the result of the deinitialization. /// - `OsalReturnType::KSuccess`: The semaphore was successfully deinitialized. /// - `OsalReturnType::KFail`: An error occurred during the deinitialization. - OsalReturnType deinit(); + [[nodiscard]] OsalReturnType deinit(); /// @brief Decrement the semaphore, blocking until the semaphore can be decremented or the timeout expires. /// This method does not use `sem_timedwait` to attempt to decrement the semaphore because that does not use a monotonic clock. @@ -85,7 +85,7 @@ class Semaphore final { /// - `OsalReturnType::KSuccess`: The semaphore was successfully decremented within the specified time. /// - `OsalReturnType::KTimeout`: The semaphore was not decremented because the wait timed out. /// - `OsalReturnType::KFail`: An error occurred during the wait operation (e.g., if the system clock could not be read). - OsalReturnType timedWait(std::chrono::milliseconds delay); + [[nodiscard]] OsalReturnType timedWait(std::chrono::milliseconds delay); /// @brief Increments (posts) the semaphore. /// This method uses `sem_post` to increment the semaphore: @@ -96,7 +96,7 @@ class Semaphore final { /// @return An OsalReturnType indicating the result of the operation. /// - `OsalReturnType::KSuccess`: The semaphore was successfully incremented (posted). /// - `OsalReturnType::KFail`: An error occurred during the increment (post) operation. - OsalReturnType post(); + [[nodiscard]] OsalReturnType post(); /// @brief Decrements (waits) the semaphore. /// This method uses `sem_wait` to decrement the semaphore: @@ -107,7 +107,7 @@ class Semaphore final { /// @return An OsalReturnType indicating the result of the operation. /// - `OsalReturnType::KSuccess`: The semaphore was successfully decremented (waited). /// - `OsalReturnType::KFail`: An error occurred during the decrement (wait) operation. - OsalReturnType wait(); + [[nodiscard]] OsalReturnType wait(); private: /// @brief POSIX semaphore object diff --git a/score/launch_manager/src/daemon/src/osal/set_affinity.hpp b/score/launch_manager/src/daemon/src/osal/set_affinity.hpp index ad5614bd6..1a396901e 100644 --- a/score/launch_manager/src/daemon/src/osal/set_affinity.hpp +++ b/score/launch_manager/src/daemon/src/osal/set_affinity.hpp @@ -36,7 +36,7 @@ namespace osal { /// currently physically on the system and permitted to the /// thread according to any restrictions that may be imposed /// elsewhere. -int32_t setaffinity(uint32_t cpumask) noexcept(true); +[[nodiscard]] int32_t setaffinity(uint32_t cpumask) noexcept(true); } // namespace osal } // namespace lcm } // namespace internal diff --git a/score/launch_manager/src/daemon/src/osal/set_groups.hpp b/score/launch_manager/src/daemon/src/osal/set_groups.hpp index d8b8efe82..a784ce525 100644 --- a/score/launch_manager/src/daemon/src/osal/set_groups.hpp +++ b/score/launch_manager/src/daemon/src/osal/set_groups.hpp @@ -32,7 +32,7 @@ namespace osal { /// to the underlying OS call. /// @param __groups pointer to the list of groups, may be NULL /// @returns 0 on success, -1 on failure. -std::int32_t setgroups(size_t __n, const gid_t *__groups) noexcept(true); +[[nodiscard]] std::int32_t setgroups(size_t __n, const gid_t *__groups) noexcept(true); } // namespace osal } // namespace lcm } // namespace internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp index 426c94163..86f426fb9 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp @@ -282,7 +282,7 @@ void ProcessInfoNode::terminated(int32_t process_status) // handle the situation where a worker thread is waiting for a process to terminate if (has_semaphore_.exchange(false)) { - terminator_.post(); + static_cast(terminator_.post()); } } @@ -519,7 +519,7 @@ inline void ProcessInfoNode::handleTerminationProcess() { auto pg_mgr = graph_->getProcessGroupManager(); - terminator_.init(0U, false); + static_cast(terminator_.init(0U, false)); has_semaphore_.store(true); LM_LOG_DEBUG() << "Requesting termination of process" << process_index_ << "of" << graph_->getProcessGroupName() << "pid" << pid_ << "(" << config_->startup_config_.short_name_ << ")"; @@ -538,7 +538,7 @@ inline void ProcessInfoNode::handleTerminationProcess() } has_semaphore_.store(false); - terminator_.deinit(); + static_cast(terminator_.deinit()); } inline void ProcessInfoNode::handleForcedTermination() diff --git a/score/launch_manager/src/lifecycle_client/src/details/report_running_impl.cpp b/score/launch_manager/src/lifecycle_client/src/details/report_running_impl.cpp index 57b548c44..c6789d8e8 100644 --- a/score/launch_manager/src/lifecycle_client/src/details/report_running_impl.cpp +++ b/score/launch_manager/src/lifecycle_client/src/details/report_running_impl.cpp @@ -105,7 +105,12 @@ namespace score::mw::lifecycle { } // Final post to semaphore, so LM know that communication channel can be closed now - sync->send_sync_.post(); + if (sync->send_sync_.post() == OsalReturnType::kFail) + { + LM_LOG_ERROR() << "[Lifecycle Client] Final synchronization post failed."; + + return comms_error; + } // Mark as reported if successful reported = true; // Set return value to success From 2463ee1bd77ab5fc841ce9ff086f90c550a55170 Mon Sep 17 00:00:00 2001 From: Sumitkumar Satpute Date: Wed, 1 Jul 2026 14:15:17 +0200 Subject: [PATCH 2/3] Address review comments: -use assert and ASSERT_TRUE safeguards Signed-off-by: Sumitkumar Satpute sumitkumar.sa.satpute@bti.bmwgroup.com --- .../src/control_client/src/details/control_client_impl.cpp | 5 ++++- .../src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/score/launch_manager/src/control_client/src/details/control_client_impl.cpp b/score/launch_manager/src/control_client/src/details/control_client_impl.cpp index 6ab243ddc..218b539cb 100644 --- a/score/launch_manager/src/control_client/src/details/control_client_impl.cpp +++ b/score/launch_manager/src/control_client/src/details/control_client_impl.cpp @@ -12,6 +12,7 @@ ********************************************************************************/ #include +#include #include #include #include @@ -94,7 +95,9 @@ ControlClientImpl::ControlClientImpl(std::function(ipc_request_semaphore_.init(1U, false)); + const auto init_result = ipc_request_semaphore_.init(1U, false); + assert((score::lcm::internal::osal::OsalReturnType::kSuccess == init_result) && + "ControlClient semaphore initialization failed"); ipc_response_thread_ = std::make_unique(&ControlClientImpl::run, this); } diff --git a/score/launch_manager/src/daemon/src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp b/score/launch_manager/src/daemon/src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp index 9fd808f6e..5eb5ccb7c 100644 --- a/score/launch_manager/src/daemon/src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp +++ b/score/launch_manager/src/daemon/src/alive_monitor/details/ifappl/MonitorIfDaemon_UT.cpp @@ -74,7 +74,8 @@ struct MonitorIfDaemonFixture /// Initialize the IPC server so that peek/pop/hasOverflow use real shared memory. void initIpc() { - static_cast(ipcServer.init(makeUniqueIpcName())); + ASSERT_TRUE(ipcServer.init(makeUniqueIpcName()) == ifappl::CheckpointIpcServer::EIpcInitResult::kOk) + << "CheckpointIpcServer init failed"; } /// Drive the process to the 'running' state and notify observers. From 1f6b0a4b355c558f7b078b637fac23554e9dba91 Mon Sep 17 00:00:00 2001 From: Sumitkumar Satpute Date: Wed, 1 Jul 2026 14:35:40 +0200 Subject: [PATCH 3/3] Address review comments: -use SCORE Assert Macro Signed-off-by: Sumitkumar Satpute sumitkumar.sa.satpute@bti.bmwgroup.com --- .../control_client/src/details/control_client_impl.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/score/launch_manager/src/control_client/src/details/control_client_impl.cpp b/score/launch_manager/src/control_client/src/details/control_client_impl.cpp index 218b539cb..8bf051d5f 100644 --- a/score/launch_manager/src/control_client/src/details/control_client_impl.cpp +++ b/score/launch_manager/src/control_client/src/details/control_client_impl.cpp @@ -12,11 +12,12 @@ ********************************************************************************/ #include -#include #include #include #include +#include + #include "score/concurrency/future/interruptible_future.h" #include "score/concurrency/future/interruptible_promise.h" @@ -96,8 +97,9 @@ ControlClientImpl::ControlClientImpl(std::function(&ControlClientImpl::run, this); }