Coverity- Medium and low priority issues (#524)

common/source/LinuxUtils.cpp

@@ -24,32 +24,41 @@
 #include <pwd.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <vector>
 
 namespace
 {
-constexpr uid_t kNoOwnerChange = -1; // -1 means chown() won't change the owner
-constexpr gid_t kNoGroupChange = -1; // -1 means chown() won't change the group
+constexpr uid_t kNoOwnerChange = -1;        // -1 means chown() won't change the owner
+constexpr gid_t kNoGroupChange = -1;        // -1 means chown() won't change the group
+constexpr size_t kDefaultBufferSize = 4096; // Fallback buffer size
 
 uid_t getFileOwnerId(const std::string &fileOwner)
 {
     uid_t ownerId = kNoOwnerChange;
-    // sysconf returns long; -1 on error. Store as int64_t to avoid unsigned conversion issues.
-    const int64_t bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
-    if (!fileOwner.empty() && bufferSizeLong > 0)
+    if (!fileOwner.empty())
     {
-        const size_t kBufferSize = static_cast<size_t>(bufferSizeLong);
+        const int64_t bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
+        const size_t kBufferSize = (bufferSizeLong > 0) ? static_cast<size_t>(bufferSizeLong) : kDefaultBufferSize;
+
         errno = 0;
         passwd passwordStruct{};
         passwd *passwordResult = nullptr;
-        char buffer[kBufferSize];
-        int result = getpwnam_r(fileOwner.c_str(), &passwordStruct, buffer, kBufferSize, &passwordResult);
-        if (result == 0 && passwordResult)
+        std::vector<char> buffer(kBufferSize);
+        int result = getpwnam_r(fileOwner.c_str(), &passwordStruct, buffer.data(), buffer.size(), &passwordResult);
+        if (result == 0)
         {
-            ownerId = passwordResult->pw_uid;
+            if (passwordResult)
+            {
+                ownerId = passwordResult->pw_uid;
+            }
+            else
+            {
+                RIALTO_COMMON_LOG_WARN("Owner name '%s' not found", fileOwner.c_str());
+            }
         }
         else
         {
-            RIALTO_COMMON_LOG_SYS_WARN(errno, "Failed to determine ownerId for '%s'", fileOwner.c_str());
+            RIALTO_COMMON_LOG_SYS_WARN(result, "Failed to lookup ownerId for '%s'", fileOwner.c_str());
         }
     }
     return ownerId;
@@ -58,23 +67,30 @@ uid_t getFileOwnerId(const std::string &fileOwner)
 gid_t getFileGroupId(const std::string &fileGroup)
 {
     gid_t groupId = kNoGroupChange;
-    // sysconf returns long; -1 on error. Store as int64_t to avoid unsigned conversion issues.
-    const int64_t bufferSizeLong = sysconf(_SC_GETGR_R_SIZE_MAX);
-    if (!fileGroup.empty() && bufferSizeLong > 0)
+    if (!fileGroup.empty())
     {
-        const size_t kBufferSize = static_cast<size_t>(bufferSizeLong);
+        const int64_t bufferSizeLong = sysconf(_SC_GETGR_R_SIZE_MAX);
+        const size_t kBufferSize = (bufferSizeLong > 0) ? static_cast<size_t>(bufferSizeLong) : kDefaultBufferSize;
+
         errno = 0;
         group groupStruct{};
         group *groupResult = nullptr;
-        char buffer[kBufferSize];
-        int result = getgrnam_r(fileGroup.c_str(), &groupStruct, buffer, kBufferSize, &groupResult);
-        if (result == 0 && groupResult)
+        std::vector<char> buffer(kBufferSize);
+        int result = getgrnam_r(fileGroup.c_str(), &groupStruct, buffer.data(), buffer.size(), &groupResult);
+        if (result == 0)
         {
-            groupId = groupResult->gr_gid;
+            if (groupResult)
+            {
+                groupId = groupResult->gr_gid;
+            }
+            else
+            {
+                RIALTO_COMMON_LOG_WARN("Group name '%s' not found", fileGroup.c_str());
+            }
         }
         else
         {
-            RIALTO_COMMON_LOG_SYS_WARN(errno, "Failed to determine groupId for '%s'", fileGroup.c_str());
+            RIALTO_COMMON_LOG_SYS_WARN(result, "Failed to lookup groupId for '%s'", fileGroup.c_str());
         }
     }
     return groupId;

ipc/common/source/NamedSocket.cpp

@@ -29,11 +29,13 @@
 #include <sys/un.h>
 #include <unistd.h>
 #include <utility>
+#include <vector>
 
 namespace
 {
-constexpr uid_t kNoOwnerChange = -1; // -1 means chown() won't change the owner
-constexpr gid_t kNoGroupChange = -1; // -1 means chown() won't change the group
+constexpr uid_t kNoOwnerChange = -1;        // -1 means chown() won't change the owner
+constexpr gid_t kNoGroupChange = -1;        // -1 means chown() won't change the group
+constexpr size_t kDefaultBufferSize = 4096; // Fallback buffer size
 } // namespace
 
 namespace firebolt::rialto::ipc
@@ -269,23 +271,30 @@ bool NamedSocket::getSocketLock()
 uid_t NamedSocket::getSocketOwnerId(const std::string &socketOwner) const
 {
     uid_t ownerId = kNoOwnerChange;
-    // sysconf returns long; -1 on error. Store as int64_t to avoid unsigned conversion issues.
-    const int64_t bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
-    if (!socketOwner.empty() && bufferSizeLong > 0)
+    if (!socketOwner.empty())
     {
-        const size_t kBufferSize = static_cast<size_t>(bufferSizeLong);
+        const int64_t bufferSizeLong = sysconf(_SC_GETPW_R_SIZE_MAX);
+        const size_t kBufferSize = (bufferSizeLong > 0) ? static_cast<size_t>(bufferSizeLong) : kDefaultBufferSize;
+
         errno = 0;
         passwd passwordStruct{};
         passwd *passwordResult = nullptr;
-        char buffer[kBufferSize];
-        int result = getpwnam_r(socketOwner.c_str(), &passwordStruct, buffer, kBufferSize, &passwordResult);
-        if (result == 0 && passwordResult)
+        std::vector<char> buffer(kBufferSize);
+        int result = getpwnam_r(socketOwner.c_str(), &passwordStruct, buffer.data(), buffer.size(), &passwordResult);
+        if (result == 0)
         {
-            ownerId = passwordResult->pw_uid;
+            if (passwordResult)
+            {
+                ownerId = passwordResult->pw_uid;
+            }
+            else
+            {
+                RIALTO_IPC_LOG_WARN("Owner name '%s' not found", socketOwner.c_str());
+            }
         }
         else
         {
-            RIALTO_IPC_LOG_SYS_WARN(errno, "Failed to determine ownerId for '%s'", socketOwner.c_str());
+            RIALTO_IPC_LOG_SYS_WARN(result, "Failed to lookup ownerId for '%s'", socketOwner.c_str());
         }
     }
     return ownerId;
@@ -294,23 +303,30 @@ uid_t NamedSocket::getSocketOwnerId(const std::string &socketOwner) const
 gid_t NamedSocket::getSocketGroupId(const std::string &socketGroup) const
 {
     gid_t groupId = kNoGroupChange;
-    // sysconf returns long; -1 on error. Store as int64_t to avoid unsigned conversion issues.
-    const int64_t bufferSizeLong = sysconf(_SC_GETGR_R_SIZE_MAX);
-    if (!socketGroup.empty() && bufferSizeLong > 0)
+    if (!socketGroup.empty())
     {
-        const size_t kBufferSize = static_cast<size_t>(bufferSizeLong);
+        const int64_t bufferSizeLong = sysconf(_SC_GETGR_R_SIZE_MAX);
+        const size_t kBufferSize = (bufferSizeLong > 0) ? static_cast<size_t>(bufferSizeLong) : kDefaultBufferSize;
+
         errno = 0;
         group groupStruct{};
         group *groupResult = nullptr;
-        char buffer[kBufferSize];
-        int result = getgrnam_r(socketGroup.c_str(), &groupStruct, buffer, kBufferSize, &groupResult);
-        if (result == 0 && groupResult)
+        std::vector<char> buffer(kBufferSize);
+        int result = getgrnam_r(socketGroup.c_str(), &groupStruct, buffer.data(), buffer.size(), &groupResult);
+        if (result == 0)
         {
-            groupId = groupResult->gr_gid;
+            if (groupResult)
+            {
+                groupId = groupResult->gr_gid;
+            }
+            else
+            {
+                RIALTO_IPC_LOG_WARN("Group name '%s' not found", socketGroup.c_str());
+            }
         }
         else
         {
-            RIALTO_IPC_LOG_SYS_WARN(errno, "Failed to determine groupId for '%s'", socketGroup.c_str());
+            RIALTO_IPC_LOG_SYS_WARN(result, "Failed to lookup groupId for '%s'", socketGroup.c_str());
         }
     }
     return groupId;

media/public/include/IMediaPipeline.h

@@ -543,7 +543,7 @@ class IMediaPipeline
          *
          * @retval the media key session id.
          */
-        const int32_t getMediaKeySessionId() const { return m_mediaKeySessionId; }
+        int32_t getMediaKeySessionId() const { return m_mediaKeySessionId; }
 
         /**
          * @brief Returns the key id. Empty if unencrypted.
@@ -571,14 +571,14 @@ class IMediaPipeline
          *
          * @retval the initWithLast15 value.
          */
-        const uint32_t getInitWithLast15() const { return m_initWithLast15; }
+        uint32_t getInitWithLast15() const { return m_initWithLast15; }
 
         /**
          * @brief Returns the segment alignment
          *
          * @retval the segment alignment
          */
-        const SegmentAlignment getSegmentAlignment() const { return m_alignment; }
+        SegmentAlignment getSegmentAlignment() const { return m_alignment; }
 
         /**
          * @brief Gets the codec data
@@ -602,7 +602,7 @@ class IMediaPipeline
          *
          * @retval if the encryption pattern has been set
          */
-        const bool getEncryptionPattern(uint32_t &crypt, uint32_t &skip) const
+        bool getEncryptionPattern(uint32_t &crypt, uint32_t &skip) const
         {
             crypt = m_crypt;
             skip = m_skip;

media/server/gstplayer/include/WebAudioPlayerContext.h

@@ -66,6 +66,11 @@ struct WebAudioPlayerContext
      */
     uint32_t lastBytesWritten{};
 
+    /**
+     * @brief Counter that increments each time a write operation completes.
+     */
+    uint32_t writeCompletionCounter{};
+
     /**
      * @brief The number of bytes per sample.
      */

media/server/gstplayer/source/GstGenericPlayer.cpp

@@ -2037,7 +2037,7 @@ bool GstGenericPlayer::setSyncOff()
 
             if (m_glibWrapper->gObjectClassFindProperty(G_OBJECT_GET_CLASS(decoder), "sync-off"))
             {
-                gboolean syncOffGboolean{decoder ? TRUE : FALSE};
+                gboolean syncOffGboolean{syncOff ? TRUE : FALSE};
                 m_glibWrapper->gObjectSet(decoder, "sync-off", syncOffGboolean, nullptr);
                 result = true;
             }
@@ -2135,8 +2135,13 @@ bool GstGenericPlayer::setRenderFrame()
                 RIALTO_SERVER_LOG_INFO("Rendering preroll");
 
                 m_glibWrapper->gObjectSet(sink, kStepOnPrerollPropertyName.c_str(), 1, nullptr);
-                m_gstWrapper->gstElementSendEvent(sink, m_gstWrapper->gstEventNewStep(GST_FORMAT_BUFFERS, 1, 1.0, true,
-                                                                                      false));
+                gboolean sendRet =
+                    m_gstWrapper->gstElementSendEvent(sink, m_gstWrapper->gstEventNewStep(GST_FORMAT_BUFFERS, 1, 1.0,
+                                                                                          true, false));
+                if (!sendRet)
+                {
+                    RIALTO_SERVER_LOG_WARN("Failed to send step event for rendering preroll frame");
+                }
                 m_glibWrapper->gObjectSet(sink, kStepOnPrerollPropertyName.c_str(), 0, nullptr);
                 result = true;
             }

media/server/gstplayer/source/GstWebAudioPlayer.cpp

@@ -410,9 +410,13 @@ uint32_t GstWebAudioPlayer::writeBuffer(uint8_t *mainPtr, uint32_t mainLength, u
 {
     // Must block and wait for the data to be written from the shared buffer.
     std::unique_lock<std::mutex> lock(m_context.writeBufferMutex);
+    const uint32_t initialCompletionCounter = m_context.writeCompletionCounter;
     m_workerThread->enqueueTask(m_taskFactory->createWriteBuffer(m_context, mainPtr, mainLength, wrapPtr, wrapLength));
-    std::cv_status status = m_context.writeBufferCond.wait_for(lock, std::chrono::milliseconds(kMaxWriteBufferTimeoutMs));
-    if (std::cv_status::timeout == status)
+    bool success =
+        m_context.writeBufferCond.wait_for(lock, std::chrono::milliseconds(kMaxWriteBufferTimeoutMs),
+                                           [this, initialCompletionCounter]()
+                                           { return m_context.writeCompletionCounter != initialCompletionCounter; });
+    if (!success)
     {
         RIALTO_SERVER_LOG_ERROR("Timed out writing to the gstreamer buffers");
         return 0;
@@ -470,4 +474,4 @@ void GstWebAudioPlayer::ping(std::unique_ptr<IHeartbeatHandler> &&heartbeatHandl
     }
 }
 
-}; // namespace firebolt::rialto::server
+} // namespace firebolt::rialto::server

media/server/gstplayer/source/tasks/webAudio/WriteBuffer.cpp

@@ -102,6 +102,7 @@ void WriteBuffer::execute() const
     {
         std::unique_lock<std::mutex> lock(m_context.writeBufferMutex);
         m_context.lastBytesWritten = bytesWritten;
+        ++m_context.writeCompletionCounter;
     }
     m_context.writeBufferCond.notify_one();
 }

media/server/ipc/source/MediaPipelineModuleService.cpp

@@ -429,10 +429,10 @@ void MediaPipelineModuleService::attachSource(::google::protobuf::RpcController
     std::shared_ptr<CodecData> codecData{};
     if (request->has_codec_data())
     {
-        auto codecDataProto = request->codec_data();
+        const auto &kCodecDataProto = request->codec_data();
         codecData = std::make_shared<CodecData>();
-        codecData->data = std::vector<std::uint8_t>(codecDataProto.data().begin(), codecDataProto.data().end());
-        codecData->type = convertCodecDataType(codecDataProto.type());
+        codecData->data = std::vector<std::uint8_t>(kCodecDataProto.data().begin(), kCodecDataProto.data().end());
+        codecData->type = convertCodecDataType(kCodecDataProto.type());
     }
     std::unique_ptr<IMediaPipeline::MediaSource> mediaSource;
     firebolt::rialto::SourceConfigType configType = convertConfigType(request->config_type());

media/server/main/interface/IMainThread.h

@@ -20,7 +20,6 @@
 #ifndef FIREBOLT_RIALTO_SERVER_I_MAIN_THREAD_H_
 #define FIREBOLT_RIALTO_SERVER_I_MAIN_THREAD_H_
 
-#include "IMainThread.h"
 #include <functional>
 #include <memory>
 #include <utility>

media/server/main/source/MainThread.cpp

@@ -112,9 +112,9 @@ const std::shared_ptr<MainThread::TaskInfo> MainThread::waitForTask()
     {
         m_taskQueueCv.wait(lock, [this] { return !m_taskQueue.empty(); });
     }
-    const auto kTaskInfo = m_taskQueue.front();
+    auto taskInfo = std::move(m_taskQueue.front());
     m_taskQueue.pop_front();
-    return kTaskInfo;
+    return taskInfo;
 }
 
 int32_t MainThread::registerClient()

serverManager/common/source/SessionServerAppManager.cpp

@@ -491,7 +491,7 @@ bool SessionServerAppManager::configureSessionServerWithSocketName(const std::sh
     const auto kSocketPermissions{kSessionServer->getSessionManagementSocketPermissions()};
     const auto kSocketOwner{kSessionServer->getSessionManagementSocketOwner()};
     const auto kSocketGroup{kSessionServer->getSessionManagementSocketGroup()};
-    const auto kAppName{kSessionServer->getAppName()};
+    const auto &kAppName{kSessionServer->getAppName()};
 
     const firebolt::rialto::common::MaxResourceCapabilitites kMaxResource{kSessionServer->getMaxPlaybackSessions(),
                                                                           kSessionServer->getMaxWebAudioPlayers()};
@@ -513,7 +513,7 @@ bool SessionServerAppManager::configureSessionServerWithSocketFd(const std::shar
     const auto kInitialState{kSessionServer->getInitialState()};
     const auto kSocketFd{kSessionServer->getSessionManagementSocketFd()};
     const auto kClientDisplayName{kSessionServer->getClientDisplayName()};
-    const auto kAppName{kSessionServer->getAppName()};
+    const auto &kAppName{kSessionServer->getAppName()};
 
     const firebolt::rialto::common::MaxResourceCapabilitites kMaxResource{kSessionServer->getMaxPlaybackSessions(),
                                                                           kSessionServer->getMaxWebAudioPlayers()};