From 237d0a8db3e70c68d48dda0c4e31ada73e070f19 Mon Sep 17 00:00:00 2001 From: Grzegorz Rynkowski Date: Wed, 20 Feb 2013 15:31:15 +0100 Subject: [PATCH] Fix warnings shown by cppcheck for wrt-commons [Issue#] LINUXWRT-126 [Problem] Cppcheck appears some warnings for wrt-commons. [Cause] Incorrect code. [Solution] Make some changes(not 3rdparty). [Verification] Run cppcheck on wrt-plugins-common (with --suppress=incorrectStringBooleanError) or check cppcheck.txt generated by build bot (ignore warnigs like "A boolean comparison with the string literal ... is always true"). Ignore warnings about 3rdparty. Change-Id: I150192e3f096307bf1a4a4cc2a12f6cf0c787d11 --- examples/fake_rpc/fake_rpc.cpp | 8 +++++ modules/db/src/naive_synchronization_object.cpp | 6 +++- modules/localization/src/LanguageTagsProvider.cpp | 2 +- modules/test/src/test_results_collector.cpp | 2 +- modules/utils/src/file_utils.cpp | 42 +++++++++++++---------- modules/utils/src/wrt_global_settings.cpp | 2 +- modules/widget_dao/dao/widget_dao.cpp | 7 ++-- modules/widget_dao/dao/widget_dao_read_only.cpp | 6 +++- tests/core/test_fast_delegate.cpp | 11 ++++++ tests/core/test_serialization.cpp | 3 +- tests/dao/TestCases_PluginDAO.cpp | 6 ++-- tests/db/test_orm.cpp | 14 ++++---- tests/event/test_controller.cpp | 6 +++- tests/unused/test_shm.cpp | 20 +++++++---- tests/unused/test_sql_connection.cpp | 6 ++-- tests/utils/wrt_utility.cpp | 16 +++++++-- 16 files changed, 107 insertions(+), 50 deletions(-) diff --git a/examples/fake_rpc/fake_rpc.cpp b/examples/fake_rpc/fake_rpc.cpp index 3396a59..82a2443 100644 --- a/examples/fake_rpc/fake_rpc.cpp +++ b/examples/fake_rpc/fake_rpc.cpp @@ -102,6 +102,14 @@ private: } public: + MyThread() : + m_rpcUnixClient(NULL), + m_rpcFakeClient(NULL), + m_connections(0), + m_sentData(0), + m_receivedData(0) + {} + virtual ~MyThread() { // Always quit thread diff --git a/modules/db/src/naive_synchronization_object.cpp b/modules/db/src/naive_synchronization_object.cpp index 0fbc386..1ac71ca 100644 --- a/modules/db/src/naive_synchronization_object.cpp +++ b/modules/db/src/naive_synchronization_object.cpp @@ -24,12 +24,16 @@ #include #include +namespace { + unsigned int seed = time(NULL); +} + namespace DPL { namespace DB { void NaiveSynchronizationObject::Synchronize() { // Sleep for about 10ms - 30ms - Thread::MiliSleep(10 + rand() % 20); + Thread::MiliSleep(10 + rand_r(&seed) % 20); } void NaiveSynchronizationObject::NotifyAll() diff --git a/modules/localization/src/LanguageTagsProvider.cpp b/modules/localization/src/LanguageTagsProvider.cpp index fcf2077..0d97e16 100644 --- a/modules/localization/src/LanguageTagsProvider.cpp +++ b/modules/localization/src/LanguageTagsProvider.cpp @@ -139,12 +139,12 @@ void LanguageTagsProvider::createTagsFromLocales(const char* language) DPL::String langdescr = LocaleToBCP47LanguageTag(DPL::FromUTF8String(language)); - size_t position; if (langdescr.empty()) { LogError("Empty language description while correct value needed"); } else { /* Language tags list should not be cleared before this place to * avoid losing current data when new data are invalid */ + size_t position; while (true) { LogDebug("Processing language description: " << langdescr); m_languageTagsList.push_back(langdescr); diff --git a/modules/test/src/test_results_collector.cpp b/modules/test/src/test_results_collector.cpp index 482b132..025dd84 100644 --- a/modules/test/src/test_results_collector.cpp +++ b/modules/test/src/test_results_collector.cpp @@ -48,7 +48,7 @@ const char *DEFAULT_XML_FILE_NAME = "results.xml"; bool ParseCollectorFileArg(const std::string &arg, std::string &filename) { const std::string argname = "--file="; - if (0 == arg.find(argname)) { + if (arg.find(argname) == 0 ) { filename = arg.substr(argname.size()); return true; } diff --git a/modules/utils/src/file_utils.cpp b/modules/utils/src/file_utils.cpp index 2510069..c6e77e2 100644 --- a/modules/utils/src/file_utils.cpp +++ b/modules/utils/src/file_utils.cpp @@ -89,28 +89,32 @@ int RmDir(const char* path) return -1; } - struct dirent* entry = NULL; - do { + int return_code; + struct dirent entry = NULL; + struct dirent* entry_result; + + for (return_code = readdir_r(dir, &entry, &entry_result) != 0; + entry_result != NULL && return_code != 0; + return_code = readdir_r(dir, &entry, &entry_result)) + { errno = 0; - if (NULL != (entry = ::readdir(dir))) { - if (!::strncmp(entry->d_name, ".", 1) || - !::strncmp(entry->d_name, "..", 2)) - { - continue; - } - std::string fullPath = WrtDB::PathBuilder(path) - .Append(entry->d_name) - .GetFullPath(); - if (RmNode(fullPath.c_str()) != 0) { - int error = errno; - TEMP_FAILURE_RETRY(::closedir(dir)); - errno = error; - return -1; - } + if (!::strncmp(entry.d_name, ".", 1) || + !::strncmp(entry.d_name, "..", 2)) + { + continue; } - } while (NULL != entry); + std::string fullPath = WrtDB::PathBuilder(path) + .Append(entry.d_name) + .GetFullPath(); + if (RmNode(fullPath.c_str()) != 0) { + int error = errno; + TEMP_FAILURE_RETRY(::closedir(dir)); + errno = error; + return -1; + } + } - int error = errno; + int error = (!errno) ? errno : return_code; if (TEMP_FAILURE_RETRY(::closedir(dir)) != 0) { return -1; } diff --git a/modules/utils/src/wrt_global_settings.cpp b/modules/utils/src/wrt_global_settings.cpp index adaac78..4f005c5 100644 --- a/modules/utils/src/wrt_global_settings.cpp +++ b/modules/utils/src/wrt_global_settings.cpp @@ -91,9 +91,9 @@ bool initializeGlobalSettings() // ignore environment variables if this flag is not set #ifdef GLOBAL_SETTINGS_CONTROL char * envStr = getenv(WRT_TEST_MODE); - int testMode = 0; if (NULL != envStr) { std::string env = envStr; + int testMode = 0; if ("1" == env) { testMode = ALL_TEST; } else { diff --git a/modules/widget_dao/dao/widget_dao.cpp b/modules/widget_dao/dao/widget_dao.cpp index 65ea027..d67b408 100644 --- a/modules/widget_dao/dao/widget_dao.cpp +++ b/modules/widget_dao/dao/widget_dao.cpp @@ -36,6 +36,10 @@ #include #include +namespace { + unsigned int seed = time(NULL); +} + namespace WrtDB { //TODO in current solution in each getter there exists a check //"IsWidgetInstalled". Maybe it should be verified, if it could be done @@ -244,10 +248,9 @@ DbWidgetHandle WidgetDAO::registerWidget( //make it more precise due to very fast tests struct timeval tv; gettimeofday(&tv, NULL); - srand(time(NULL) + tv.tv_usec); DbWidgetHandle widgetHandle; do { - widgetHandle = rand(); + widgetHandle = rand_r(&seed); } while (isWidgetInstalled(widgetHandle)); registerWidget(*pWidgetRegisterInfo.configInfo.tizenAppId, diff --git a/modules/widget_dao/dao/widget_dao_read_only.cpp b/modules/widget_dao/dao/widget_dao_read_only.cpp index f8fdf0b..5f7274b 100644 --- a/modules/widget_dao/dao/widget_dao_read_only.cpp +++ b/modules/widget_dao/dao/widget_dao_read_only.cpp @@ -37,6 +37,10 @@ #include #include +namespace { + unsigned int seed = time(NULL); +} + namespace WrtDB { //TODO in current solution in each getter there exists a check //"IsWidgetInstalled". Maybe it should be verified, if it could be done @@ -1185,7 +1189,7 @@ TizenPkgId WidgetDAOReadOnly::generatePkgId() pkgId.resize(MAX_TIZENID_LENGTH); do { for (int i = 0; i < MAX_TIZENID_LENGTH; ++i) { - pkgId[i] = allowed[rand() % allowed.length()]; + pkgId[i] = allowed[rand_r(&seed) % allowed.length()]; } } while (isWidgetInstalled(pkgId)); return pkgId; diff --git a/tests/core/test_fast_delegate.cpp b/tests/core/test_fast_delegate.cpp index dc9b34a..c773c4e 100644 --- a/tests/core/test_fast_delegate.cpp +++ b/tests/core/test_fast_delegate.cpp @@ -96,6 +96,10 @@ class COtherClass double rubbish; // to ensure this class has non-zero size. public: + COtherClass() : + rubbish(0) + {} + virtual ~COtherClass() {} @@ -106,6 +110,13 @@ class COtherClass class VeryBigClass { + public: + VeryBigClass() { + memset(letsMakeThingsComplicated, 0, + 400 * sizeof(letsMakeThingsComplicated[0])); + } + + private: int letsMakeThingsComplicated[400]; }; diff --git a/tests/core/test_serialization.cpp b/tests/core/test_serialization.cpp index 202589b..7bbf8de 100644 --- a/tests/core/test_serialization.cpp +++ b/tests/core/test_serialization.cpp @@ -70,7 +70,8 @@ class TestClass : public DPL::ISerializable c.push_back(str2); c.push_back(str1 + str2); } - TestClass(DPL::IStream& stream) + TestClass(DPL::IStream& stream) : + a(0) //TODO: consider the need (g.rynkowski) { DPL::Deserialization::Deserialize(stream, a); DPL::Deserialization::Deserialize(stream, b); diff --git a/tests/dao/TestCases_PluginDAO.cpp b/tests/dao/TestCases_PluginDAO.cpp index 6c73a7b..5254a2b 100644 --- a/tests/dao/TestCases_PluginDAO.cpp +++ b/tests/dao/TestCases_PluginDAO.cpp @@ -140,8 +140,8 @@ RUNNER_TEST(plugin_dao_test_register_library_dependencies) PluginHandle depHandles[] = { 117, 119 }; PluginHandleSetPtr dependencies(new PluginHandleSet); + dependencies->insert(depHandles[0]); dependencies->insert(depHandles[1]); - dependencies->insert(depHandles[2]); PluginDAO::registerPluginLibrariesDependencies(handle, dependencies); @@ -153,9 +153,9 @@ RUNNER_TEST(plugin_dao_test_register_library_dependencies) retDependencies->size() == sizeof(depHandles) / sizeof(depHandles[0])); RUNNER_ASSERT( - retDependencies->find(depHandles[1]) != retDependencies->end()); + retDependencies->find(depHandles[0]) != retDependencies->end()); RUNNER_ASSERT( - retDependencies->find(depHandles[2]) != retDependencies->end()); + retDependencies->find(depHandles[1]) != retDependencies->end()); } } diff --git a/tests/db/test_orm.cpp b/tests/db/test_orm.cpp index 500f43d..c7c9ea9 100644 --- a/tests/db/test_orm.cpp +++ b/tests/db/test_orm.cpp @@ -589,7 +589,7 @@ RUNNER_TEST(ORM_Delete) // properly for (std::list::iterator i = originalList.begin(); i != originalList.end(); - i++) + ++i) { TestTableDelete::Insert insert(interface.get()); insert.Values(*i); @@ -1095,14 +1095,14 @@ RUNNER_TEST(ORM_SelectOrderByMultipleColumns) DPL::FromASCIIString( "test 6"), "Wrong row 1 order"); RUNNER_ASSERT_MSG(iter->Get_TestID() == 10, "Wrong row 1 order"); - iter++; + ++iter; } { //2 row RUNNER_ASSERT_MSG(*iter->Get_TestText33() == DPL::FromASCIIString( "test 5"), "Wrong row 2 order"); RUNNER_ASSERT_MSG(iter->Get_TestID() == 7, "Wrong row 2 order"); - iter++; + ++iter; } { //3 row RUNNER_ASSERT_MSG(iter->Get_Value3() == 111, "Wrong row 3 order"); @@ -1110,7 +1110,7 @@ RUNNER_TEST(ORM_SelectOrderByMultipleColumns) DPL::FromASCIIString( "test 2"), "Wrong row 3 order"); RUNNER_ASSERT_MSG(iter->Get_TestID() == 2, "Wrong row 3 order"); - iter++; + ++iter; } { //4 row RUNNER_ASSERT_MSG(iter->Get_Value3() == 111, "Wrong row 4 order"); @@ -1118,7 +1118,7 @@ RUNNER_TEST(ORM_SelectOrderByMultipleColumns) DPL::FromASCIIString( "test 1"), "Wrong row 4 order"); RUNNER_ASSERT_MSG(iter->Get_TestID() == 1, "Wrong row 4 order"); - iter++; + ++iter; } { //5 row RUNNER_ASSERT_MSG(iter->Get_Value3() == 222, "Wrong row 5 order"); @@ -1126,7 +1126,7 @@ RUNNER_TEST(ORM_SelectOrderByMultipleColumns) DPL::FromASCIIString( "test 4"), "Wrong row 5 order"); RUNNER_ASSERT_MSG(iter->Get_TestID() == 6, "Wrong row 5 order"); - iter++; + ++iter; } { //6 row RUNNER_ASSERT_MSG(iter->Get_Value3() == 222, "Wrong row 6 order"); @@ -1134,7 +1134,7 @@ RUNNER_TEST(ORM_SelectOrderByMultipleColumns) DPL::FromASCIIString( "test 3"), "Wrong row 6 order"); RUNNER_ASSERT_MSG(iter->Get_TestID() == 3, "Wrong row 6 order"); - iter++; + ++iter; } } } diff --git a/tests/event/test_controller.cpp b/tests/event/test_controller.cpp index 8f0a630..5308720 100644 --- a/tests/event/test_controller.cpp +++ b/tests/event/test_controller.cpp @@ -34,6 +34,10 @@ RUNNER_TEST_GROUP_INIT(DPL) +namespace { + unsigned int seed = time(NULL); +} + class IntController : public DPL::Event::Controller::Type> { @@ -375,7 +379,7 @@ class TestController : return; } ++testContextPtr->g_SentCounter; - int id = rand() % static_cast(testContextPtr->controllers.size()); + int id = rand_r(&seed) % static_cast(testContextPtr->controllers.size()); testContextPtr->controllers.at(id)->DPL::Event:: ControllerEventHandler::PostEvent(RandomEvent()); } diff --git a/tests/unused/test_shm.cpp b/tests/unused/test_shm.cpp index 4f2d93a..a5c6046 100644 --- a/tests/unused/test_shm.cpp +++ b/tests/unused/test_shm.cpp @@ -558,7 +558,10 @@ class EnumTestSO1 : public TestSharedObject protected: explicit EnumTestSO1(const std::string& semaphore) : - TestSharedObject(semaphore) {} + TestSharedObject(semaphore), + m_waitable(NULL) + {} + virtual void PropertyChanged(size_t propertyEnum); @@ -1065,7 +1068,7 @@ RUNNER_TEST(SharedMemory_070_SharedObjectListeners) RUNNER_ASSERT(g_values[2] == 3); int array[64]; - memset(array, 64 * sizeof(array[0]), 0); + memset(array, 0, 64 * sizeof(array[0])); array[5] = 5; sho->SetProperty<3, int[64]>(array); Wait(waitable); @@ -1172,6 +1175,11 @@ RUNNER_TEST(SharedMemory_090_SetPropertyDelegate) */ class LazySharedObject : public SharedObject { + private: + LazySharedObject() : + m_read(false) + {} + public: explicit LazySharedObject(const std::string& semaphore) : SharedObject(semaphore) @@ -1391,7 +1399,8 @@ class ShmController4 : public ListeningController, }; explicit ShmController4(DPL::WaitableEvent* event) : - ListeningController(event) + ListeningController(event), + m_counter(0) {} virtual void OnEventReceived(const int& event); @@ -1412,6 +1421,7 @@ class ShmController4 : public ListeningController, void Sleep(); size_t m_counter; + static unsigned int seed = time(NULL); }; void ShmController4::ValueChanged(size_t propertyEnum, @@ -1449,7 +1459,7 @@ void ShmController4::ValueChanged(size_t propertyEnum, void ShmController4::Sleep() { DPL::Thread::GetCurrentThread()->MiliSleep( - rand() % MAX_SINGLETON_LISTENER_DELAY); + rand_r(&seed) % MAX_SINGLETON_LISTENER_DELAY); } void ShmController4::OnEventReceived(const int& event) @@ -1531,8 +1541,6 @@ RUNNER_TEST(SharedMemory_130_SharedObjectSingleton) { RemoveIpcs(); // we need non existing shm - srand(time(NULL)); - // writer shared object TestSharedObjectPtr sho = SharedObjectFactory::Create( SHM_KEY, SEM_NAME); diff --git a/tests/unused/test_sql_connection.cpp b/tests/unused/test_sql_connection.cpp index 4525c98..bc2b7e0 100644 --- a/tests/unused/test_sql_connection.cpp +++ b/tests/unused/test_sql_connection.cpp @@ -31,11 +31,11 @@ RUNNER_TEST(SqlConnection_MassiveReadWrite_SemaphoreSynchronization) { - srand(time(NULL)); - std::ostringstream dbSemaporeFileNameStream; + unsigned int seed = time(NULL); + dbSemaporeFileNameStream << "dpl_tests_dbso_sem_"; - dbSemaporeFileNameStream << rand() << ".sem"; + dbSemaporeFileNameStream << rand_r(&seed) << ".sem"; std::string dbSemaphoreFileName = dbSemaporeFileNameStream.str(); diff --git a/tests/utils/wrt_utility.cpp b/tests/utils/wrt_utility.cpp index eb78ed6..710b578 100644 --- a/tests/utils/wrt_utility.cpp +++ b/tests/utils/wrt_utility.cpp @@ -89,16 +89,26 @@ RUNNER_TEST(wrt_utility_WrtUtilMakeDir) RUNNER_TEST(wrt_utility_WrtUtilMakeDir_PermissionError) { if (0 == getuid()) { + int bufsize; + if ((bufsize = sysconf(_SC_GETPW_R_SIZE_MAX)) == -1) + RUNNER_ASSERT_MSG(false, + "Getting an initial value suggested for the size of buffer failed."); + //Change UID to execute the test correctly errno = 0; - struct passwd *p = getpwnam("app"); - if (p == NULL) { + char *buffer = new char[bufsize]; + struct passwd p; + struct passwd *result = NULL; + int return_value = getpwnam_r("app", &p, buffer, bufsize, &result); + delete[] buffer; + + if (return_value != 0 || !result) { int error = errno; RUNNER_ASSERT_MSG(false, "Getting app user UID failed: " << (error == 0 ? "No error detected" : strerror(error))); } - if (setuid(p->pw_uid) != 0) { + if (setuid(p.pw_uid) != 0) { int error = errno; RUNNER_ASSERT_MSG(false, "Changing to app user's UID failed: " << (error == -- 2.7.4