RDKEMW-12282: Fix Coverity identified issues - dobby

AppInfrastructure/Common/include/ConditionVariable.h

@@ -188,7 +188,6 @@ class ConditionVariable
         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

@@ -44,7 +44,6 @@ void ThreadedDispatcher::post(std::function<void ()> work)
     if(running)
     {
         q.push_back(work);
-        lock.unlock();
         cv.notify_one();
     }
     else
@@ -74,7 +73,6 @@ void syncCallback(std::mutex* lock, std::condition_variable* cond, bool* fired)
     std::unique_lock<std::mutex> locker(*lock);
     *fired = true;
     cond->notify_all();
-    locker.unlock();
 }
 } // namespace
 /**
@@ -117,7 +115,6 @@ void ThreadedDispatcher::sync()
     }
     // 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,34 +123,40 @@ void ThreadedDispatcher::sync()
         cond.wait(locker);
     }
 }
-namespace
-{
-void unlockAndSetFlagToFalse(std::mutex& m, bool& flag)
-{
-    using namespace std;
-    m.unlock();
-    flag = false;
-}
-}
+
 /**
  * @brief Perform any work remaining in the queue, then stop accepting new work.
  */
 void ThreadedDispatcher::flush()
 {
     //To ensure all the work that is in the queue is done, we lock a mutex.
-    //post a job to the queue that unlocks it and stops running further jobs.
+    //post a job to the queue that signals completion via condition variable.
     //Then block here until that's done.
+    std::unique_lock<std::mutex> runningLocker(m);
     if(running)
     {
-        std::mutex m2;
-        m2.lock();
-        post(bind(unlockAndSetFlagToFalse, std::ref(m2), std::ref(this->running)));
-        m2.lock();
-        m2.unlock();
+        runningLocker.unlock();
+        std::mutex flushMutex;
+        std::condition_variable flushCond;
+        bool flushed = false;
+
+        std::unique_lock<std::mutex> locker(flushMutex);
+        post([&]() {
+            std::lock_guard<std::mutex> lock(flushMutex);
+            flushed = true;
+            running = false;
+            flushCond.notify_one();
+        });
+
+        while (!flushed) {
+            flushCond.wait(locker);
+        }
+
         stop();
     }
     else
     {
+        runningLocker.unlock();
         AI_LOG_WARN("This dispatcher is no longer running. Ignoring flush request.");
     }
 }
@@ -164,7 +167,6 @@ void ThreadedDispatcher::stop()
 {
     std::unique_lock<std::mutex> lock(m);
     running = false;
-    lock.unlock();
     cv.notify_one();
     t.join();
 }
@@ -214,4 +216,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

@@ -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
@@ -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

@@ -195,7 +195,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/source/DobbyBundleConfig.cpp

@@ -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;
 }
 
@@ -286,6 +289,7 @@ const std::map<std::string, Json::Value>& DobbyBundleConfig::legacyPlugins() con
 
 const std::map<std::string, Json::Value>& DobbyBundleConfig::rdkPlugins() const
 {
+    std::lock_guard<std::mutex> locker(mLock);
     return mRdkPlugins;
 }
 

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)
     {

bundle/lib/source/DobbySpecConfig.cpp

@@ -917,8 +917,8 @@ 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;
 
     if (mSpecVersion == SpecVersion::Version1_0)
     {

bundle/lib/source/DobbyTemplate.cpp

@@ -124,9 +124,10 @@ DobbyTemplate* DobbyTemplate::instance()
         }
     }
 
+    DobbyTemplate* result = mInstance;
     pthread_rwlock_unlock(&mInstanceLock);
 
-    return mInstance;
+    return result;
 }
 
 // -----------------------------------------------------------------------------
@@ -536,7 +537,7 @@ std::string DobbyTemplate::_apply(const ctemplate::TemplateDictionaryInterface*
 
     if (!mTemplateCache->ExpandNoLoad(mTemplateKey,
                                       prettyPrint ? ctemplate::STRIP_WHITESPACE
-                                                  : ctemplate::STRIP_WHITESPACE,
+                                                  : ctemplate::DO_NOT_STRIP,
                                       dictionary, nullptr, &result))
     {
         AI_LOG_ERROR("template cache expand on load failed");
@@ -654,7 +655,7 @@ bool DobbyTemplate::_applyAt(int dirFd, const std::string& fileName,
     bool success = mTemplateCache->ExpandNoLoad(mTemplateKey,
                                                 prettyPrint ?
                                                     ctemplate::STRIP_WHITESPACE :
-                                                    ctemplate::STRIP_WHITESPACE,
+                                                    ctemplate::DO_NOT_STRIP,
                                                 dictionary, nullptr, &emitter);
     if (!success)
     {

bundle/tool/source/Main.cpp

@@ -113,7 +113,6 @@ static void parseArgs(const int argc, char **argv)
                 fprintf(stderr, "Warning: Unknown option `-%c'.\n", optopt);
             else
                 fprintf(stderr, "Warning: Unknown option character `\\x%x'.\n", optopt);
-
         default:
             exit(EXIT_FAILURE);
             break;
@@ -228,57 +227,62 @@ static bool generateOciBundle(std::shared_ptr<IDobbySettings> settings,
  */
 int main(int argc, char *argv[])
 {
-    printf("Dobby Bundle Generator Tool\n");
-    parseArgs(argc, argv);
+    try {
+        printf("Dobby Bundle Generator Tool\n");
+        parseArgs(argc, argv);
 
-    // Set up so we can read commands
-    auto readLine = IReadLine::create();
-    if (!readLine || !readLine->isValid())
-    {
-        AI_LOG_ERROR_EXIT("failed to create ReadLine object");
-        exit(EXIT_FAILURE);
-    }
+        // Set up so we can read commands
+        auto readLine = IReadLine::create();
+        if (!readLine || !readLine->isValid())
+        {
+            AI_LOG_ERROR_EXIT("failed to create ReadLine object");
+            exit(EXIT_FAILURE);
+        }
 
-    // Can't do any work without a file to process or somewhere to put the
-    // output
-    if (inputPath.empty())
-    {
-        AI_LOG_ERROR("Must provide a Dobby spec as an input");
-        exit(EXIT_FAILURE);
-    }
-    else if (access(inputPath.c_str(), R_OK) != 0)
-    {
-        AI_LOG_ERROR("Cannot access Dobby spec file %s", inputPath.c_str());
-        exit(EXIT_FAILURE);
-    }
+        // Can't do any work without a file to process or somewhere to put the
+        // output
+        if (inputPath.empty())
+        {
+            AI_LOG_ERROR("Must provide a Dobby spec as an input");
+            exit(EXIT_FAILURE);
+        }
+        else if (access(inputPath.c_str(), R_OK) != 0)
+        {
+            AI_LOG_ERROR("Cannot access Dobby spec file %s", inputPath.c_str());
+            exit(EXIT_FAILURE);
+        }
 
-    // We'll create the directory if it's missing later, so no need to check
-    // if we can read it
-    if (outputDirectory.empty())
-    {
-        AI_LOG_ERROR("Must provide an output directory");
-        exit(EXIT_FAILURE);
-    }
+        // We'll create the directory if it's missing later, so no need to check
+        // if we can read it
+        if (outputDirectory.empty())
+        {
+            AI_LOG_ERROR("Must provide an output directory");
+            exit(EXIT_FAILURE);
+        }
 
-    // Dobby uses a JSON settings file to provide STB-specific settings (e.g GPU)
-    // Can be left blank (defaylt settings will be used)
-    else if (!settingsPath.empty() && access(settingsPath.c_str(), R_OK) != 0)
-    {
-        AI_LOG_ERROR("Cannot access settings file %s", inputPath.c_str());
-        exit(EXIT_FAILURE);
-    }
+        // Dobby uses a JSON settings file to provide STB-specific settings (e.g GPU)
+        // Can be left blank (defaylt settings will be used)
+        else if (!settingsPath.empty() && access(settingsPath.c_str(), R_OK) != 0)
+        {
+            AI_LOG_ERROR("Cannot access settings file %s", inputPath.c_str());
+            exit(EXIT_FAILURE);
+        }
 
-    AI_LOG_INFO("Parsing Dobby spec file %s\n", inputPath.c_str());
-    AI_LOG_INFO("Generating Bundle in directory: %s\n", outputDirectory.c_str());
+        AI_LOG_INFO("Parsing Dobby spec file %s\n", inputPath.c_str());
+        AI_LOG_INFO("Generating Bundle in directory: %s\n", outputDirectory.c_str());
 
-    // Get settings from the provided json file
-    auto settings = readSettings();
-    auto utils = std::make_shared<DobbyUtils>();
+        // Get settings from the provided json file
+        auto settings = readSettings();
+        auto utils = std::make_shared<DobbyUtils>();
 
-    // Now we can do some actual work
-    generateOciBundle(settings, utils, inputPath, outputDirectory);
+        // Now we can do some actual work
+        generateOciBundle(settings, utils, inputPath, outputDirectory);
 
-    // And we're done
-    AICommon::termLogging();
-    return EXIT_SUCCESS;
-}
+        // And we're done
+        AICommon::termLogging();
+        return EXIT_SUCCESS;
+        } catch (const std::exception& e) {
+            AI_LOG_ERROR_EXIT("Unhandled exception in main: %s\n", e.what());
+            exit(EXIT_FAILURE);
+	}
+}

client/lib/source/DobbyProxy.cpp

@@ -136,7 +136,6 @@ DobbyProxy::~DobbyProxy()
     {
         std::unique_lock<std::mutex> locker(mStateChangeLock);
         mStateChangeQueue.emplace_back(StateChangeEvent::Terminate);
-        locker.unlock();
 
         mStateChangeCond.notify_all();
         mStateChangeThread.join();