RDKEMW-12282: Fix Coverity identified issues - dobby

AppInfrastructure/Common/include/ConditionVariable.h

@@ -181,14 +181,13 @@ class ConditionVariable
         {
             return std::cv_status::no_timeout;
         }
-	else if (err == ETIMEDOUT)
+        else if (err == ETIMEDOUT)
         {
             return std::cv_status::timeout;
         }
         else
         {
             __ConditionVariableThrowOnError(err);
-            return std::cv_status::timeout;
         }
     }
 

AppInfrastructure/Common/include/IDGenerator.h

@@ -25,6 +25,7 @@
 
 #include <bitset>
 #include <mutex>
+#include <random>
 
 #include <math.h>
 #include <stdlib.h>
@@ -85,10 +86,17 @@ class IDGenerator
                                         (N == 19) ? 0x4032F :
                                         (N == 20) ? 0x80534 : 0;
 
+private:
+    static unsigned getRandomSeed()
+    {
+        std::random_device rd;
+        return rd();
+    }
+
 public:
     IDGenerator(unsigned offset = 0)
         : mOffset(offset)
-        , mLfsr(1 + (rand() % (mSize- 2)))
+        , mLfsr(1 + (getRandomSeed() % (mSize - 2)))
     { }
 
 public:

AppInfrastructure/Common/source/ThreadedDispatcher.cpp

@@ -40,20 +40,18 @@ ThreadedDispatcher::ThreadedDispatcher(int priority, const std::string& name /*=
 }
 void ThreadedDispatcher::post(std::function<void ()> work)
 {
-    std::unique_lock<std::mutex> lock(m);
-    if(running)
-    {
+    {std::unique_lock<std::mutex> lock(m);
+        if(!running)
+        {
+            AI_LOG_WARN("Ignoring work because the dispatcher is not running anymore");
+            return;
+            //can't throw an exception here because if this is executed from destructor,
+            //which occurs when work adds more work things go horribly wrong.
+            //Instead, ignore work.
+        }
         q.push_back(work);
-        lock.unlock();
-        cv.notify_one();
-    }
-    else
-    {
-        AI_LOG_WARN("Ignoring work because the dispatcher is not running anymore");
-        //can't throw an exception here because if this is executed from destructor,
-        //which occurs when work adds more work things go horribly wrong.
-        //Instead, ignore work.
     }
+    cv.notify_one();
 }
 namespace
 {
@@ -71,10 +69,10 @@ namespace
  */
 void syncCallback(std::mutex* lock, std::condition_variable* cond, bool* fired)
 {
-    std::unique_lock<std::mutex> locker(*lock);
-    *fired = true;
+    {std::unique_lock<std::mutex> locker(*lock);
+        *fired = true;
+    }
     cond->notify_all();
-    locker.unlock();
 }
 } // namespace
 /**
@@ -85,7 +83,7 @@ bool ThreadedDispatcher::invokedFromDispatcherThread()
     bool res = (std::this_thread::get_id() == t.get_id());
     if (res)
     {
-		std::stringstream ss;
+        std::stringstream ss;
         ss << "Caller thread Id [" << std::this_thread::get_id() << "] == [dispatcher thread Id " << t.get_id() << "]";
         AI_LOG_ERROR("%s", ss.str().c_str());
     }
@@ -109,15 +107,15 @@ void ThreadedDispatcher::sync()
     std::condition_variable cond;
     bool fired = false;
     // Take the queue lock and ensure we're still running
-    std::unique_lock<std::mutex> qlocker(m);
-    if (!running)
-    {
-        AI_LOG_DEBUG("Ignoring sync because dispatcher is not running");
-        return;
+    {std::unique_lock<std::mutex> qlocker(m);
+        if (!running)
+        {
+            AI_LOG_DEBUG("Ignoring sync because dispatcher is not running");
+            return;
+        }
+        // Add the work object to the queue which takes the lock and sets 'fired' to true
+        q.push_back(std::bind(syncCallback, &lock, &cond, &fired));
     }
-    // Add the work object to the queue which takes the lock and sets 'fired' to true
-    q.push_back(std::bind(syncCallback, &lock, &cond, &fired));
-    qlocker.unlock();
     cv.notify_one();
     // Wait for 'fired' to become true
     std::unique_lock<std::mutex> locker(lock);
@@ -126,6 +124,7 @@ void ThreadedDispatcher::sync()
         cond.wait(locker);
     }
 }
+
 namespace
 {
 void unlockAndSetFlagToFalse(std::mutex& m, bool& flag)
@@ -148,6 +147,7 @@ void ThreadedDispatcher::flush()
         std::mutex m2;
         m2.lock();
         post(bind(unlockAndSetFlagToFalse, std::ref(m2), std::ref(this->running)));
+        // coverity[double_lock : FALSE]
         m2.lock();
         m2.unlock();
         stop();
@@ -157,14 +157,15 @@ void ThreadedDispatcher::flush()
         AI_LOG_WARN("This dispatcher is no longer running. Ignoring flush request.");
     }
 }
+
 /**
  * @brief Cancels any work that is not already in progress, stop accepting new work
  */
 void ThreadedDispatcher::stop()
 {
-    std::unique_lock<std::mutex> lock(m);
-    running = false;
-    lock.unlock();
+    {std::unique_lock<std::mutex> lock(m);
+        running = false;
+    }
     cv.notify_one();
     t.join();
 }
@@ -214,4 +215,4 @@ std::function<void ()> ThreadedDispatcher::next()
     q.pop_front();
     return work;
 }
-} //AICommon
+} //AICommon

AppInfrastructure/Common/source/Timer.cpp

@@ -32,7 +32,10 @@ using namespace AICommon;
 
 Timer::~Timer()
 {
-    cancel();
+    try {
+        cancel();
+    } catch (const std::exception& e) {
+    }
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

AppInfrastructure/IpcService/source/sdbus/SDBusIpcService.cpp

@@ -180,9 +180,9 @@ SDBusIpcService::~SDBusIpcService()
 
         // invoke the quit lambda
         if (!runOnEventLoopThread(std::move(quitExec)))
-	{
-		AI_LOG_ERROR("Failed to post quitExec to event loop thread");
-	}
+        {
+            AI_LOG_ERROR("Failed to post quitExec to event loop thread");
+        }
 
 
         // wait for the thread to quit
@@ -223,7 +223,7 @@ bool SDBusIpcService::init(const std::string &serviceName,
     if (defaultTimeoutMs <= 0)
         mDefaultTimeoutUsecs = (25 * 1000 * 1000);
     else
-        mDefaultTimeoutUsecs = (defaultTimeoutMs * 1000);
+        mDefaultTimeoutUsecs = (static_cast<uint64_t>(defaultTimeoutMs) * 1000);
 
 
     // eventfd used to wake the poll loop
@@ -344,8 +344,8 @@ bool SDBusIpcService::stop()
 
     if (!runOnEventLoopThread(std::move(nopExec)))
     {
-	    AI_LOG_ERROR("Failed to queue noop event on event loop thread");
-     }
+        AI_LOG_ERROR("Failed to queue noop event on event loop thread");
+    }
 
 
     return true;
@@ -426,7 +426,7 @@ std::shared_ptr<IAsyncReplyGetter> SDBusIpcService::invokeMethod(const Method &m
     if (timeoutMs < 0)
         timeoutUsecs = mDefaultTimeoutUsecs;
     else
-        timeoutUsecs = (timeoutMs * 1000);
+        timeoutUsecs = (static_cast<uint64_t>(timeoutMs) * 1000);
 
     // create the reply getter
     std::shared_ptr<SDBusAsyncReplyGetter> replyGetter =
@@ -504,7 +504,7 @@ bool SDBusIpcService::invokeMethod(const Method &method,
     if (timeoutMs < 0)
         timeoutUsecs = mDefaultTimeoutUsecs;
     else
-        timeoutUsecs = (timeoutMs * 1000);
+        timeoutUsecs = (static_cast<uint64_t>(timeoutMs) * 1000);
 
     // clear the reply args list
     replyArgs.clear();

AppInfrastructure/Public/Common/Notifier.h

@@ -185,6 +185,7 @@ class Notifier : virtual public Polymorphic
         {
             // We are unregistering an observer so make sure we will not notify unregistered observers
             lock.unlock();
+            /* coverity[missing_lock : FALSE] */
             dispatcher->sync();
             lock.lock();
         }
@@ -195,7 +196,6 @@ class Notifier : virtual public Polymorphic
         {
             cv.notify_all();
         }
-        lock.unlock();
     }
 
 protected:

AppInfrastructure/ReadLine/source/ReadLine.cpp

@@ -438,7 +438,7 @@ void ReadLine::commandExecute(const std::string& cmdStr,
     }
     else if (!errStr.empty())
     {
-        fprintf(stderr, "Ambiguous command '\%s', possible commands: %s %s\n",
+        fprintf(stderr, "Ambiguous command '%s', possible commands: %s %s\n",
                 cmdStr.c_str(), cmdRef->name.c_str(), errStr.c_str());
     }
     else if (cmdRef->handler != nullptr)

bundle/lib/include/DobbyBundleConfig.h

@@ -79,7 +79,7 @@ class DobbyBundleConfig : public DobbyConfig
     std::shared_ptr<rt_dobby_schema> config() const override;
 
 public:
-    const std::map<std::string, Json::Value>& rdkPlugins() const override;
+   const std::map<std::string, Json::Value>& rdkPlugins() const override;
 
 #if defined(LEGACY_COMPONENTS)
 public:

bundle/lib/source/DobbyBundleConfig.cpp

@@ -81,9 +81,9 @@ DobbyBundleConfig::DobbyBundleConfig(const std::shared_ptr<IDobbyUtils>& utils,
             std::string postInstallPath = bundlePath + "/postinstallhooksuccess";
 
             if (remove(postInstallPath.c_str()) != 0)  
-	    {
-		    AI_LOG_ERROR("Failed to remove postinstallhooksuccess");
-	    }
+            {
+                AI_LOG_ERROR("Failed to remove postinstallhooksuccess");
+            }
 
 
             // Retry creation of config
@@ -164,11 +164,13 @@ bool DobbyBundleConfig::isValid() const
 
 uid_t DobbyBundleConfig::userId() const
 {
+    std::lock_guard<std::mutex> locker(mLock);
     return mUserId;
 }
 
 gid_t DobbyBundleConfig::groupId() const
 {
+    std::lock_guard<std::mutex> locker(mLock);
     return mGroupId;
 }
 
@@ -244,6 +246,7 @@ const std::string& DobbyBundleConfig::rootfsPath() const
 
 bool DobbyBundleConfig::restartOnCrash() const
 {
+    std::lock_guard<std::mutex> locker(mLock);
     return mRestartOnCrash;
 }
 

bundle/lib/source/DobbyConfig.cpp

@@ -320,6 +320,7 @@ bool DobbyConfig::changeProcessArgs(const std::string& command)
  */
 void DobbyConfig::printCommand() const
 {
+    std::lock_guard<std::mutex> locker(mLock);
     std::shared_ptr<rt_dobby_schema> cfg = config();
     if (cfg == nullptr)
     {
@@ -557,7 +558,7 @@ bool DobbyConfig::writeConfigJsonImpl(const std::string& filePath) const
     // set file permissions
     if (chmod(filePath.c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) != 0)
     {
-	    AI_LOG_SYS_WARN(errno, "Failed to set permissions on config file '%s'", filePath.c_str());
+        AI_LOG_SYS_WARN(errno, "Failed to set permissions on config file '%s'", filePath.c_str());
     }
 
 

bundle/lib/source/DobbyRootfs.cpp

@@ -142,14 +142,23 @@ DobbyRootfs::DobbyRootfs(const std::shared_ptr<IDobbyUtils>& utils,
     }
 
     // check that rootfs exists in bundle
-    if (access(rootfsDirPath.c_str(), F_OK) == -1)
+    int dirFd = open(rootfsDirPath.c_str(), O_CLOEXEC | O_DIRECTORY);
+    if (dirFd == -1)
     {
-        AI_LOG_ERROR_EXIT("could not find rootfs at %s", rootfsDirPath.c_str());
-        return;
+        if (errno == ENOENT)
+        {
+            AI_LOG_ERROR_EXIT("could not find rootfs at %s", rootfsDirPath.c_str());
+            return;
+        }
+        else
+        {
+            AI_LOG_SYS_ERROR(errno, "failed to open rootfs directory '%s'", rootfsDirPath.c_str());
+            return;
+        }
     }
     else
     {
-        mDirFd = open(rootfsDirPath.c_str(), O_CLOEXEC | O_DIRECTORY);
+        mDirFd = dirFd;
     }
 
     // store the complete path

bundle/lib/source/DobbySpecConfig.cpp

@@ -917,9 +917,10 @@ bool DobbySpecConfig::processUserNs(const Json::Value& value,
 bool DobbySpecConfig::processRtPriority(const Json::Value& value,
                                         ctemplate::TemplateDictionary* dictionary)
 {
-    int rtPriorityDefault;
-    int rtPriorityLimit;
+    int rtPriorityDefault = 0;
+    int rtPriorityLimit = 0;
 
+    std::lock_guard<std::mutex> locker(mLock);
     if (mSpecVersion == SpecVersion::Version1_0)
     {
         if (!value.isIntegral())

bundle/lib/source/DobbyTemplate.cpp

@@ -124,9 +124,10 @@ DobbyTemplate* DobbyTemplate::instance()
         }
     }
 
+    DobbyTemplate* result = mInstance;
     pthread_rwlock_unlock(&mInstanceLock);
 
-    return mInstance;
+    return result;
 }
 
 // -----------------------------------------------------------------------------