Fixed findings from static code analysis

9 files changed, 149 insertions, 107 deletions

diff --git a/src/persistence_client_library.c b/src/persistence_client_library.c
index dbbe424..3d93aa8 100644
--- a/src/persistence_client_library.c
+++ b/src/persistence_client_library.c
@@ -223,7 +223,7 @@ int pclInitLibrary(const char* appName, int shutdownMode)
    int lock = pthread_mutex_lock(&gInitMutex);
    if(lock == 0)
    {
-      if(appName != NULL)
+      if(appName != NULL && strlen(appName) > 0 && strlen(appName) < 256)
       {
          if(gPclInitCounter == 0)
          {
diff --git a/src/persistence_client_library_backup_filelist.c b/src/persistence_client_library_backup_filelist.c
index 11fbe1e..d94817f 100644
--- a/src/persistence_client_library_backup_filelist.c
+++ b/src/persistence_client_library_backup_filelist.c
@@ -352,7 +352,7 @@ int pclVerifyConsistency(const char* origPath, const char* backupPath, const cha
                      {
                         DLT_LOG(gPclDLTContext, DLT_LOG_INFO, DLT_STRING("verifyConsist- csum no match csum and original"));
 
-                        if(strcmp(origCsumBuf, origCsumBuf)  != 0)
+                        if(strcmp(backCsumBuf, origCsumBuf)  != 0)
                         {
                            DLT_LOG(gPclDLTContext, DLT_LOG_INFO, DLT_STRING("verifyConsist- csum no match backup and original"));
                            close(handle);
diff --git a/src/persistence_client_library_db_access.c b/src/persistence_client_library_db_access.c
index df0ba38..45773bf 100644
--- a/src/persistence_client_library_db_access.c
+++ b/src/persistence_client_library_db_access.c
@@ -722,56 +722,64 @@ int persistence_notify_on_change(const char* resource_id, const char* dbKey, uns
 
    	// search if item is already stored in the tree
    	searchItem = malloc(sizeof(key_value_s));
-      searchItem->key = hashKey;
-      foundItem = (key_value_s*)jsw_rbfind(gNotificationTree, searchItem);
+   	if(searchItem != NULL)
+   	{
+         searchItem->key = hashKey;
+         foundItem = (key_value_s*)jsw_rbfind(gNotificationTree, searchItem);
 
-      if(regPolicy == Notify_register)
-      {
-         if(foundItem == NULL)   // item not found add it, else already added so nothing to do
+         if(regPolicy == Notify_register)
          {
-            key_value_s* item = malloc(sizeof(key_value_s));    // assign key and value to the rbtree item
-            if(item != NULL)
+            if(foundItem == NULL)   // item not found add it, else already added so nothing to do
             {
-               item->key = hashKey;
-               // we don't need the path name here, we just need to know that this key is available in the tree
-               item->value = "";
-               (void)jsw_rbinsert(gNotificationTree, item);
-               free(item);
+               key_value_s* item = malloc(sizeof(key_value_s));    // assign key and value to the rbtree item
+               if(item != NULL)
+               {
+                  item->key = hashKey;
+                  // we don't need the path name here, we just need to know that this key is available in the tree
+                  item->value = "";
+                  (void)jsw_rbinsert(gNotificationTree, item);
+                  free(item);
 
-               gChangeNotifyCallback = callback;      // assign callback
-            }
-            else
-            {
-               rval = -1;
+                  gChangeNotifyCallback = callback;      // assign callback
+               }
+               else
+               {
+                  rval = -1;
+               }
             }
+            free(foundItem);
          }
-         free(foundItem);
-      }
-      else if(regPolicy == Notify_unregister)
-      {
-         if(foundItem != NULL)   // item already in the tree remove it, if not found nothing to do
+         else if(regPolicy == Notify_unregister)
          {
-            // remove from tree
-            jsw_rberase(gNotificationTree, foundItem);
+            if(foundItem != NULL)   // item already in the tree remove it, if not found nothing to do
+            {
+               // we don't need the path name here, we just need to know that this key is available in the tree
+               jsw_rberase(gNotificationTree, foundItem);
 
-            if(jsw_rbsize(gNotificationTree) == 0)  // if no other notification is stored in the tree, remove callback
+               if(jsw_rbsize(gNotificationTree) == 0)  // if no other notification is stored in the tree, remove callback
+               {
+                  gChangeNotifyCallback = NULL;          // remove callback
+               }
+            }
+            else
             {
-               gChangeNotifyCallback = NULL;          // remove callback
+               free(foundItem);
             }
          }
-         else
+
+         if(-1 == deliverToMainloop(&data))
          {
-            free(foundItem);
+            DLT_LOG(gPclDLTContext, DLT_LOG_ERROR, DLT_STRING("notifyOnChange - Write to pipe"), DLT_INT(errno));
+            rval = -1;
          }
-      }
 
-      if(-1 == deliverToMainloop(&data))
+         free(searchItem);
+      }
+      else
       {
-         DLT_LOG(gPclDLTContext, DLT_LOG_ERROR, DLT_STRING("notifyOnChange - Write to pipe"), DLT_INT(errno));
+         DLT_LOG(gPclDLTContext, DLT_LOG_ERROR, DLT_STRING("notifyOnChange - failed to alloc memory"));
          rval = -1;
       }
-
-      free(searchItem);
    }
    else
    {
diff --git a/src/persistence_client_library_dbus_cmd.c b/src/persistence_client_library_dbus_cmd.c
index bb25328..c1c7322 100644
--- a/src/persistence_client_library_dbus_cmd.c
+++ b/src/persistence_client_library_dbus_cmd.c
@@ -231,11 +231,8 @@ void process_prepare_shutdown(unsigned int complete)
 
    if(complete > 0)
    {
-   	close_all_persistence_handle();
-   }
+      close_all_persistence_handle();
 
-   if(complete > 0)
-   {
 		for(i=0; i<PersCustomLib_LastEntry; i++)  // unload custom client libraries
 		{
 			if(gPersCustomFuncs[i].custom_plugin_deinit != NULL)
diff --git a/src/persistence_client_library_file.c b/src/persistence_client_library_file.c
index 714bd08..47a3720 100644
--- a/src/persistence_client_library_file.c
+++ b/src/persistence_client_library_file.c
@@ -107,7 +107,7 @@ int pclFileClose(int fd)
             if(permission != -1)	   // permission is here also used for range check
             {
                // check if a backup and checksum file needs to be deleted
-               if(permission != PersistencePermission_ReadOnly || permission != PersistencePermission_LastEntry)
+               if(permission != PersistencePermission_ReadOnly && permission != PersistencePermission_LastEntry)
                {
                   // remove backup file
                   if(remove(get_file_backup_path(fd)) == -1)
diff --git a/src/persistence_client_library_handle.c b/src/persistence_client_library_handle.c
index 4c0832c..0626127 100644
--- a/src/persistence_client_library_handle.c
+++ b/src/persistence_client_library_handle.c
@@ -56,40 +56,46 @@ int list_item_insert(PersList_item_s** list, int fd)
 {
    int rval = 1;
 
-   PersList_item_s *tmp = *list;
-   if(tmp != NULL)   // check if list is empty
+   if(list != NULL)
    {
-      while(tmp->next != NULL)
+      PersList_item_s *tmp = *list;
+      if(tmp != NULL)   // check if list is empty
       {
-         tmp = tmp->next;
-      }
+         while(tmp->next != NULL)
+         {
+            tmp = tmp->next;
+         }
 
-      tmp->next = (PersList_item_s*)malloc(sizeof(PersList_item_s));
+         tmp->next = (PersList_item_s*)malloc(sizeof(PersList_item_s));
 
-      if(tmp->next != NULL)
-      {
-         tmp->next->fd = fd;
-         tmp->next->next = NULL;
+         if(tmp->next != NULL)
+         {
+            tmp->next->fd = fd;
+            tmp->next->next = NULL;
+         }
+         else
+         {
+            rval = -1;
+         }
       }
       else
       {
-         rval = -1;
+         *list = (PersList_item_s *)malloc(sizeof(PersList_item_s));
+         if(list != NULL)
+         {
+            (*list)->fd = fd;
+            (*list)->next = NULL;
+         }
+         else
+         {
+            rval = -1;
+         }
       }
    }
    else
    {
-      *list = (PersList_item_s *)malloc(sizeof(PersList_item_s));
-      if(list != NULL)
-      {
-         (*list)->fd = fd;
-         (*list)->next = NULL;
-      }
-      else
-      {
-         rval = -1;
-      }
+      rval = -1;
    }
-
    return rval;
 }
 
diff --git a/src/persistence_client_library_prct_access.c b/src/persistence_client_library_prct_access.c
index a983735..5bd9ac6 100644
--- a/src/persistence_client_library_prct_access.c
+++ b/src/persistence_client_library_prct_access.c
@@ -224,9 +224,9 @@ int get_db_context(PersistenceInfo_s* dbContext, const char* resource_id, unsign
          dbContext->configKey.type  = PersistenceResourceType_key;
       }
 
-      memcpy(dbContext->configKey.customID, "A_CUSTOM_ID", strlen("A_CUSTOM_ID"));
-      memcpy(dbContext->configKey.reponsible, "default", strlen("default"));
-      memcpy(dbContext->configKey.custom_name, "default", strlen("default"));
+      strncpy(dbContext->configKey.customID,    "A_CUSTOM_ID", strlen("A_CUSTOM_ID "));
+      strncpy(dbContext->configKey.reponsible,  "default",     strlen("default"));
+      strncpy(dbContext->configKey.custom_name, "default",     strlen("default"));
 
       DLT_LOG(gPclDLTContext, DLT_LOG_INFO, DLT_STRING("gDBCtx - create res not in PRCT => key:"), DLT_STRING(resource_id) );
 
@@ -263,12 +263,12 @@ int get_db_path_and_key(PersistenceInfo_s* dbContext, const char* resource_id, c
          if(dbContext->context.seat_no == 0)
          {
             // /User/<user_no_parameter> is added in front of the resource ID as the key string.
-            snprintf(dbKey, PERS_DB_MAX_LENGTH_KEY_NAME, "%s%d/%s", plugin_gUser, dbContext->context.user_no, resource_id);
+            snprintf(dbKey, PERS_DB_MAX_LENGTH_KEY_NAME, "%s%u/%s", plugin_gUser, dbContext->context.user_no, resource_id);
          }
          else
          {
             // /User/<user_no_parameter>/Seat/<seat_no_parameter> is added in front of the resource ID as the key string.
-            snprintf(dbKey, PERS_DB_MAX_LENGTH_KEY_NAME, "%s%d%s%d/%s", plugin_gUser, dbContext->context.user_no, plugin_gSeat, dbContext->context.seat_no, resource_id);
+            snprintf(dbKey, PERS_DB_MAX_LENGTH_KEY_NAME, "%s%u%s%u/%s", plugin_gUser, dbContext->context.user_no, plugin_gSeat, dbContext->context.seat_no, resource_id);
          }
       }
       storePolicy = PersistenceStorage_local;
@@ -283,11 +283,11 @@ int get_db_path_and_key(PersistenceInfo_s* dbContext, const char* resource_id, c
       // /User/<user_no_parameter> and /Seat/<seat_no_parameter> are add after /<LDBID parameter> if there are different than 0.
       if(dbContext->context.seat_no != 0)
       {
-         snprintf(dbKey, PERS_DB_MAX_LENGTH_KEY_NAME, "/%x%s%d%s%d/%s", dbContext->context.ldbid, plugin_gUser, dbContext->context.user_no, plugin_gSeat, dbContext->context.seat_no, resource_id);
+         snprintf(dbKey, PERS_DB_MAX_LENGTH_KEY_NAME, "/%x%s%u%s%u/%s", dbContext->context.ldbid, plugin_gUser, dbContext->context.user_no, plugin_gSeat, dbContext->context.seat_no, resource_id);
       }
       else
       {
-         snprintf(dbKey, PERS_DB_MAX_LENGTH_KEY_NAME, "/%x%s%d/%s", dbContext->context.ldbid, plugin_gUser, dbContext->context.user_no, resource_id);
+         snprintf(dbKey, PERS_DB_MAX_LENGTH_KEY_NAME, "/%x%s%u/%s", dbContext->context.ldbid, plugin_gUser, dbContext->context.user_no, resource_id);
       }
       storePolicy = PersistenceStorage_local;
    }
diff --git a/test/Makefile.am b/test/Makefile.am
index ff58198..fa27f60 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -8,6 +8,13 @@ AM_CFLAGS = $(DEPS_CFLAGS) $(CHECK_CFLAGS)
 #AM_CFLAGS = -fprofile-arcs -ftest-coverage $(DEPS_CFLAGS) $(CHECK_CFLAGS)
 endif
 
+
+localstate_DATA = data/PAS_data_benchmark.tar.gz \
+                  data/PAS_data.tar.gz
+
+# Add config file to distribution 
+EXTRA_DIST = $(localstate_DATA) 
+
 noinst_PROGRAMS = persistence_client_library_test \
                   persistence_client_library_test_file \
                   persistence_client_library_dbus_test  \
diff --git a/test/persistence_client_library_test.c b/test/persistence_client_library_test.c
index 0324cef..d9d1c03 100644
--- a/test/persistence_client_library_test.c
+++ b/test/persistence_client_library_test.c
@@ -38,7 +38,7 @@
 #include "../include/persistence_client_library.h"
 #include "../include/persistence_client_library_error_def.h"
 
-#define SKIP_MULTITHREADED_TESTS 1
+//#define SKIP_MULTITHREADED_TESTS 1
 
 #define BUF_SIZE        64
 #define NUM_OF_FILES    3
@@ -818,6 +818,9 @@ START_TEST(test_InitDeinit)
 
    pclDeinitLibrary();
 
+   fail_unless(pclInitLibrary("", shutdownReg) == EPERS_COMMON);
+   fail_unless(pclInitLibrary(NULL, shutdownReg) == EPERS_COMMON);
+   fail_unless(pclInitLibrary(gWriteBuffer2, shutdownReg) == EPERS_COMMON);
 
 }
 END_TEST
@@ -1558,9 +1561,41 @@ START_TEST(test_NoPluginFunc)
 END_TEST
 
 
+static Suite* persistenceClientLib_suite_multi()
+{
+   const char* testSuiteName = "\n\nPersistence Client Library (Key-API) - Multi";
+
+   Suite * s  = suite_create(testSuiteName);
+
+#ifdef SKIP_MULTITHREADED_TESTS
+   printf("INFO: Skipping testcase MultiThreadedRead  (%p)\n", test_MultiThreadedRead);
+   printf("INFO: Skipping testcase MultiThreadedWrite (%p)\n", test_MultiThreadedWrite);
+#else
+   TCase * tc_MultiThreadedRead = tcase_create("MultiThreadedRead");
+   tcase_add_test(tc_MultiThreadedRead, test_MultiThreadedRead);
+   tcase_set_timeout(tc_MultiThreadedRead, 20);
+
+   TCase * tc_MultiThreadedWrite = tcase_create("MultiThreadedWrite");
+   tcase_add_test(tc_MultiThreadedWrite, test_MultiThreadedWrite);
+   tcase_set_timeout(tc_MultiThreadedWrite, 20);
+#endif
+
+#ifndef SKIP_MULTITHREADED_TESTS
+   suite_add_tcase(s, tc_MultiThreadedRead);
+   tcase_add_checked_fixture(tc_MultiThreadedRead, data_setup, data_teardown);
+
+   suite_add_tcase(s, tc_MultiThreadedWrite);
+   tcase_add_checked_fixture(tc_MultiThreadedWrite, data_setup, data_teardown);
+
+#endif
+
+   return s;
+
+}
+
 static Suite * persistenceClientLib_suite()
 {
-   const char* testSuiteName = "Persistence Client Library (Key-API)";
+   const char* testSuiteName = "\n\nPersistence Client Library (Key-API)";
 
    Suite * s  = suite_create(testSuiteName);
 
@@ -1661,20 +1696,6 @@ static Suite * persistenceClientLib_suite()
    tcase_set_timeout(tc_SharedData, 10);
 
 
-#ifdef SKIP_MULTITHREADED_TESTS
-   printf("INFO: Skipping testcase MultiThreadedRead  (%p)\n", test_MultiThreadedRead);
-   printf("INFO: Skipping testcase MultiThreadedWrite (%p)\n", test_MultiThreadedWrite);
-#else
-   TCase * tc_MultiThreadedRead = tcase_create("MultiThreadedRead");
-   tcase_add_test(tc_MultiThreadedRead, test_MultiThreadedRead);
-   tcase_set_timeout(tc_MultiThreadedRead, 20);
-
-   TCase * tc_MultiThreadedWrite = tcase_create("MultiThreadedWrite");
-   tcase_add_test(tc_MultiThreadedWrite, test_MultiThreadedWrite);
-   tcase_set_timeout(tc_MultiThreadedWrite, 20);
-#endif
-
-
 #if 1
    suite_add_tcase(s, tc_NoPluginFunc);
 
@@ -1736,21 +1757,11 @@ static Suite * persistenceClientLib_suite()
 
 #endif
 
-
 #if USE_APPCHECK
-   suite_add_tcase(s, tc_ValidApplication);
+   //suite_add_tcase(s, tc_ValidApplication);
 #endif
 
 
-#ifndef SKIP_MULTITHREADED_TESTS
-   suite_add_tcase(s, tc_MultiThreadedRead);
-   tcase_add_checked_fixture(tc_MultiThreadedRead, data_setup, data_teardown);
-
-   suite_add_tcase(s, tc_MultiThreadedWrite);
-   tcase_add_checked_fixture(tc_MultiThreadedWrite, data_setup, data_teardown);
-
-#endif
-
 
 #if 0
    suite_add_tcase(s, tc_PAS_DbusInterface);
@@ -1769,7 +1780,7 @@ static Suite * persistenceClientLib_suite()
 
 int main(int argc, char *argv[])
 {
-   int nr_failed = 0;
+   int nr_failed = 0, nr_failed2 = 0;
    (void)argv;
 
    // assign application name
@@ -1799,18 +1810,31 @@ int main(int argc, char *argv[])
    }
    else
    {
-      Suite * s = persistenceClientLib_suite();
-      SRunner * sr = srunner_create(s);
-      srunner_set_fork_status(sr, CK_NOFORK);
-      srunner_set_xml(sr, "/tmp/persistenceClientLibraryTest.xml");
-      srunner_set_log(sr, "/tmp/persistenceClientLibraryTest.log");
+      Suite * sPcl = persistenceClientLib_suite();
+      Suite * sPclMulti = persistenceClientLib_suite_multi();
+
+      SRunner * srPCL = srunner_create(sPcl);
+      SRunner * srPCLMulti = srunner_create(sPclMulti);
+
+      srunner_set_fork_status(srPCL, CK_FORK);
+      srunner_set_xml(srPCL, "/tmp/persistenceClientLibraryTest.xml");
+      srunner_set_log(srPCL, "/tmp/persistenceClientLibraryTest.log");
+
+      srunner_set_fork_status(srPCLMulti, CK_NOFORK);
+      srunner_set_xml(srPCLMulti, "/tmp/persistenceClientLibraryTestMulti.xml");
+      srunner_set_log(srPCLMulti, "/tmp/persistenceClientLibraryTestMulti.log");
+
+      srunner_run_all(srPCL, CK_VERBOSE /*CK_NORMAL CK_VERBOSE CK_SUBUNIT*/);
+      srunner_run_all(srPCLMulti, CK_VERBOSE /*CK_NORMAL CK_VERBOSE CK_SUBUNIT*/);
 
-      srunner_run_all(sr, CK_VERBOSE /*CK_NORMAL CK_VERBOSE CK_SUBUNIT*/);
+      srunner_ntests_run(srPCL);
+      srunner_ntests_run(srPCLMulti);
 
-      nr_failed = srunner_ntests_failed(sr);
-      srunner_ntests_run(sr);
+      nr_failed = srunner_ntests_failed(srPCL);
+      nr_failed2 = srunner_ntests_failed(srPCLMulti);
 
-      srunner_free(sr);
+      srunner_free(srPCL);
+      srunner_free(srPCLMulti);
    }
 
    DLT_LOG(gPcltDLTContext, DLT_LOG_INFO, DLT_STRING("End of PCL test"));
@@ -1819,7 +1843,7 @@ int main(int argc, char *argv[])
    DLT_UNREGISTER_CONTEXT(gPcltDLTContext);
    DLT_UNREGISTER_APP();
 
-   return (0==nr_failed)?EXIT_SUCCESS:EXIT_FAILURE;
+   return (0==nr_failed && 0==nr_failed2)?EXIT_SUCCESS:EXIT_FAILURE;
 
 }