Static code analysis findings; mutex rework

diff --git a/include_protected/persistence_client_library_data_organization.h b/include_protected/persistence_client_library_data_organization.h
index 3c1ecde..2e6842c 100644 (file)
--- a/include_protected/persistence_client_library_data_organization.h
+++ b/include_protected/persistence_client_library_data_organization.h
@@ -24,7 +24,7 @@
 extern "C" {
 #endif
 
-#define  PERSIST_CLIENT_LIBRARY_DATA_ORGANIZATION_INTERFACE_VERSION   (0x02000000U)
+#define  PERSIST_CLIENT_LIBRARY_DATA_ORGANIZATION_INTERFACE_VERSION   (0x03000000U)
 
 #include "../include/persistence_client_library_error_def.h"
 #include "../include/persistence_client_library_key.h"
@@ -52,15 +52,15 @@ enum _PersistenceConstantDef
    ResIsFile            = 1,        /// flag to identify that resource a file
    AccessNoLock         = 1,        /// flag to indicate that access is not locked
 
-   PCLnotInitialized    = 0,        /// 
-   PCLinitialized       = 1,        ///
+   PCLnotInitialized    = 0,        /// indication if PCL is not initialized
+   PCLinitialized       = 1,        /// indication if PCL is initialized
 
-   FileClosed           = 0,
-   FileOpen             = 1,
+   FileClosed           = 0,        /// flag to identify if file will be closed
+   FileOpen             = 1,        /// flag to identify if file has been opend
 
-   NsmShutdownNormal       = 1,        /// lifecycle shutdown normal
-   NsmErrorStatus_OK       = 1,
-   NsmErrorStatus_Fail     = -1,
+   NsmShutdownNormal       = 1,     /// lifecycle shutdown normal
+   NsmErrorStatus_OK       = 1,     /// lifecycle return OK idicator
+   NsmErrorStatus_Fail     = -1,    /// lifecycle return failed indicator
 
    ChecksumBufSize         = 64,       /// max checksum size
 
@@ -100,7 +100,6 @@ enum _PersistenceConstantDef
 };
 
 
-
 /// resource configuration table name
 extern const char* gResTableCfg;
 
@@ -175,20 +174,26 @@ extern const char* gDeleteSignal;
 /// create signal string
 extern const char* gCreateSignal;
 
-/// notification key
-extern char gSendNotifykey[DbKeyMaxLen];
-extern unsigned int gSendNotifyLdbid;
-extern unsigned int gSendNotifyUserNo;
-extern unsigned int gSendNotifySeatNo;
-extern pclNotifyStatus_e gSendNotifyReason;
-
-extern char gRegNotifykey[DbKeyMaxLen];
-extern unsigned int gRegNotifyLdbid;
-extern unsigned int gRegNotifyUserNo;
-extern unsigned int gRegNotifySeatNo;
-extern PersNotifyRegPolicy_e gRegNotifyPolicy;
 
-// dbus timeout
+/**
+ * Global notification variables, will be used to pass
+ * the notification information into the mainloop.
+ */
+/// notification key string
+extern char gNotifykey[DbKeyMaxLen];
+/// notification lbid
+extern unsigned int gNotifyLdbid;
+/// notification user number
+extern unsigned int gNotifyUserNo;
+/// notification seat number
+extern unsigned int gNotifySeatNo;
+/// notification reason (created, changed, deleted)
+extern pclNotifyStatus_e      gNotifyReason;
+/// notification policy (register or unregister)
+extern PersNotifyRegPolicy_e  gNotifyPolicy;
+
+
+// dbus timeout (5 seconds)
 extern int gTimeoutMs;
 
 // dbus pending return value

diff --git a/src/persistence_client_library_backup_filelist.c b/src/persistence_client_library_backup_filelist.c
index 8061f85..727e4e5 100644 (file)
--- a/src/persistence_client_library_backup_filelist.c
+++ b/src/persistence_client_library_backup_filelist.c
@@ -85,22 +85,25 @@ void fillCharTokenArray()
 
    while(i < gConfigFileSize)
    {
-      if(1 != gCharLookup[(int)*tmpPointer])
-      {
-         *tmpPointer = 0;
-
-         // check if we are at the end of the token array
-         if(blankCount >= TOKENARRAYSIZE)
-         {
-            break;
-         }
-         gpTokenArray[blankCount] = tmpPointer+1;
-         blankCount++;
-         gTokenCounter++;
-
-      }
+      if(   ((unsigned int)*tmpPointer < 127)
+         && ((unsigned int)*tmpPointer >= 0))
+          {
+                  if(1 != gCharLookup[(unsigned int)*tmpPointer])
+                  {
+                          *tmpPointer = 0;
+
+                          // check if we are at the end of the token array
+                          if(blankCount >= TOKENARRAYSIZE)
+                          {
+                                  break;
+                          }
+                          gpTokenArray[blankCount] = tmpPointer+1;
+                          blankCount++;
+                          gTokenCounter++;
+                  }
+          }
       tmpPointer++;
-      i++;
+          i++;
    }
 }
 
@@ -153,55 +156,66 @@ void createAndStoreFileNames()
 int readBlacklistConfigFile(const char* filename)
 {
    int fd = 0,
-       status = 0;
+       status = 0,
+       rval = 0;
+
    struct stat buffer;
 
-   memset(&buffer, 0, sizeof(buffer));
-   status = stat(filename, &buffer);
-   if(status != -1)
+   if(filename != NULL)
    {
-      gConfigFileSize = buffer.st_size;
-   }
 
-   fd = open(filename, O_RDONLY);
-   if (fd == -1)
-   {
-      DLT_LOG(gDLTContext, DLT_LOG_WARN, DLT_STRING("configReader::readConfigFile ==> Error file open"), DLT_STRING(filename), DLT_STRING(strerror(errno)) );
-      return -1;
-   }
+          memset(&buffer, 0, sizeof(buffer));
+          status = stat(filename, &buffer);
+          if(status != -1)
+          {
+                 gConfigFileSize = buffer.st_size;
+          }
 
-   // check for empty file
-   if(gConfigFileSize == 0)
-   {
-      DLT_LOG(gDLTContext, DLT_LOG_WARN, DLT_STRING("configReader::readConfigFile ==> Error file size is 0:"), DLT_STRING(filename));
-      close(fd);
-      return -1;
-   }
+          fd = open(filename, O_RDONLY);
+          if (fd == -1)
+          {
+                 DLT_LOG(gDLTContext, DLT_LOG_WARN, DLT_STRING("configReader::readConfigFile ==> Error file open"), DLT_STRING(filename), DLT_STRING(strerror(errno)) );
+                 return -1;
+          }
 
-   // map the config file into memory
-   gpConfigFileMap = (char*)mmap(0, gConfigFileSize, PROT_WRITE, MAP_PRIVATE, fd, 0);
+          // check for empty file
+          if(gConfigFileSize == 0)
+          {
+                 DLT_LOG(gDLTContext, DLT_LOG_WARN, DLT_STRING("configReader::readConfigFile ==> Error file size is 0:"), DLT_STRING(filename));
+                 close(fd);
+                 return -1;
+          }
 
-   if (gpConfigFileMap == MAP_FAILED)
-   {
-      gpConfigFileMap = 0;
-      close(fd);
-      DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("configReader::readConfigFile ==> Error mapping the file:"), DLT_STRING(filename), DLT_STRING(strerror(errno)) );
+          // map the config file into memory
+          gpConfigFileMap = (char*)mmap(0, gConfigFileSize, PROT_WRITE, MAP_PRIVATE, fd, 0);
 
-      return -1;
-   }
+          if (gpConfigFileMap == MAP_FAILED)
+          {
+                 gpConfigFileMap = 0;
+                 close(fd);
+                 DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("configReader::readConfigFile ==> Error mapping the file:"), DLT_STRING(filename), DLT_STRING(strerror(errno)) );
+
+                 return -1;
+          }
 
-   // reset the token counter
-   gTokenCounter = 0;
+          // reset the token counter
+          gTokenCounter = 0;
 
-   fillCharTokenArray();
+          fillCharTokenArray();
 
-   // create filernames and store them in the tree
-   createAndStoreFileNames();
+          // create filenames and store them in the tree
+          createAndStoreFileNames();
 
-   munmap(gpConfigFileMap, gConfigFileSize);
+          munmap(gpConfigFileMap, gConfigFileSize);
 
-   close(fd);
-   return 0;
+          close(fd);
+   }
+   else
+   {
+          rval = -1;
+   }
+
+   return rval;
 }
 
 

diff --git a/src/persistence_client_library_data_organization.c b/src/persistence_client_library_data_organization.c
index 3fffe34..73d45cf 100644 (file)
--- a/src/persistence_client_library_data_organization.c
+++ b/src/persistence_client_library_data_organization.c
@@ -74,32 +74,33 @@ const char* gSharedPublicWtPathKey    = "/Data/mnt-wt/%s/Shared_Public%s";
 /// path prefix for local cached files: /Data/mnt_c/<appId>/<user>/<seat>/<resource>
 const char* gLocalCacheFilePath        = "/Data/mnt-c/%s/user/%d/seat/%d/%s";
 
+
 const char* gChangeSignal = "PersistenceResChange";
 const char* gDeleteSignal = "PersistenceResDelete";
 const char* gCreateSignal = "PersistenceResCreate";
 
-char gSendNotifykey[DbKeyMaxLen] = {0};
-unsigned int gSendNotifyLdbid  = 0;
-unsigned int gSendNotifyUserNo = 0;
-unsigned int gSendNotifySeatNo = 0;
-pclNotifyStatus_e gSendNotifyReason  = 0;
 
-char gRegNotifykey[DbKeyMaxLen] = {0};
-unsigned int gRegNotifyLdbid  = 0;
-unsigned int gRegNotifyUserNo = 0;
-unsigned int gRegNotifySeatNo = 0;
-PersNotifyRegPolicy_e gRegNotifyPolicy;
+char gNotifykey[DbKeyMaxLen] = {0};
+unsigned int gNotifyLdbid  = 0;
+unsigned int gNotifyUserNo = 0;
+unsigned int gNotifySeatNo = 0;
+pclNotifyStatus_e       gNotifyReason = 0;
+PersNotifyRegPolicy_e   gNotifyPolicy = 0;
+
 
-int gTimeoutMs = 50000;
+int gTimeoutMs = 5000;
 
 int gDbusPendingRvalue = 0;
 
+
 /// application id
 char gAppId[MaxAppNameLen] = {0};
 
+
 /// max key value data size [default 16kB]
 int gMaxKeyValDataSize = defaultMaxKeyValDataSize;
 
+
 unsigned int gPclInitialized = PCLnotInitialized;
 
 

diff --git a/src/persistence_client_library_db_access.c b/src/persistence_client_library_db_access.c
index 9932548..8877a91 100644 (file)
--- a/src/persistence_client_library_db_access.c
+++ b/src/persistence_client_library_db_access.c
@@ -610,11 +610,11 @@ int persistence_notify_on_change(char* key, unsigned int ldbid, unsigned int use
 
    if(regPolicy < Notify_lastEntry)
    {
-      snprintf(gRegNotifykey, DbKeyMaxLen, "%s", key);
-      gRegNotifyLdbid  = ldbid;     // to do: pass correct ==> JUST TESTING!!!!
-      gRegNotifyUserNo = user_no;
-      gRegNotifySeatNo = seat_no;
-      gRegNotifyPolicy = regPolicy;
+      snprintf(gNotifykey, DbKeyMaxLen, "%s", key);
+      gNotifyLdbid  = ldbid;     // to do: pass correct ==> JUST TESTING!!!!
+      gNotifyUserNo = user_no;
+      gNotifySeatNo = seat_no;
+      gNotifyReason = regPolicy;
 
       if(regPolicy == Notify_register)
       {
@@ -651,12 +651,12 @@ int pers_send_Notification_Signal(const char* key, PersistenceDbContext_s* conte
    int rval = 1;
    if(reason < pclNotifyStatus_lastEntry)
    {
-      snprintf(gSendNotifykey,  DbKeyMaxLen,       "%s", key);
+      snprintf(gNotifykey,  DbKeyMaxLen,       "%s", key);
 
-      gSendNotifyLdbid  = context->ldbid;     // to do: pass correct ==> JUST TESTING!!!!
-      gSendNotifyUserNo = context->user_no;
-      gSendNotifySeatNo = context->seat_no;
-      gSendNotifyReason = reason;
+      gNotifyLdbid  = context->ldbid;     // to do: pass correct ==> JUST TESTING!!!!
+      gNotifyUserNo = context->user_no;
+      gNotifySeatNo = context->seat_no;
+      gNotifyReason = reason;
 
       if(-1 == deliverToMainloop(CMD_SEND_NOTIFY_SIGNAL, 0,0) )
       {

diff --git a/src/persistence_client_library_dbus_cmd.c b/src/persistence_client_library_dbus_cmd.c
index cb627f9..eee0722 100644 (file)
--- a/src/persistence_client_library_dbus_cmd.c
+++ b/src/persistence_client_library_dbus_cmd.c
@@ -43,25 +43,25 @@ void process_reg_notification_signal(DBusConnection* conn)
    // add match for  c h a n g e
    snprintf(ruleChanged, DbusMatchRuleSize,
             "type='signal',interface='org.genivi.persistence.adminconsumer',member='PersistenceResChange',path='/org/genivi/persistence/adminconsumer',arg0='%s',arg1='%u',arg2='%u',arg3='%u'",
-            gRegNotifykey, gRegNotifyLdbid, gRegNotifyUserNo, gRegNotifySeatNo);
+            gNotifykey, gNotifyLdbid, gNotifyUserNo, gNotifySeatNo);
 
    // add match for  d e l e t e
    snprintf(ruleDeleted, DbusMatchRuleSize,
             "type='signal',interface='org.genivi.persistence.adminconsumer',member='PersistenceResDelete',path='/org/genivi/persistence/adminconsumer',arg0='%s',arg1='%u',arg2='%u',arg3='%u'",
-            gRegNotifykey, gRegNotifyLdbid, gRegNotifyUserNo, gRegNotifySeatNo);
+            gNotifykey, gNotifyLdbid, gNotifyUserNo, gNotifySeatNo);
 
    // add match for  c r e a t e
    snprintf(ruleCreated, DbusMatchRuleSize,
             "type='signal',interface='org.genivi.persistence.adminconsumer',member='PersistenceResCreate',path='/org/genivi/persistence/adminconsumer',arg0='%s',arg1='%u',arg2='%u',arg3='%u'",
-            gRegNotifykey, gRegNotifyLdbid, gRegNotifyUserNo, gRegNotifySeatNo);
+            gNotifykey, gNotifyLdbid, gNotifyUserNo, gNotifySeatNo);
 
-   if(gRegNotifyPolicy == Notify_register)
+   if(gNotifyPolicy == Notify_register)
    {
       dbus_bus_add_match(conn, ruleChanged, NULL);
       dbus_bus_add_match(conn, ruleDeleted, NULL);
       dbus_bus_add_match(conn, ruleCreated, NULL);
    }
-   else if(gRegNotifyPolicy == Notify_unregister)
+   else if(gNotifyPolicy == Notify_unregister)
    {
       dbus_bus_remove_match(conn, ruleChanged, NULL);
       dbus_bus_remove_match(conn, ruleDeleted, NULL);
@@ -85,9 +85,9 @@ void process_send_notification_signal(DBusConnection* conn)
    char* pldbidArra = ldbidArray;
    char* puserArray = userArray;
    char* pseatArray = seatArray;
-   char* pnotifyKey = gSendNotifykey;
+   char* pnotifyKey = gNotifykey;
 
-   switch(gSendNotifyReason)
+   switch(gNotifyReason)
    {
       case pclNotifyStatus_deleted:
          notifyReason = gDeleteSignal;
@@ -108,9 +108,9 @@ void process_send_notification_signal(DBusConnection* conn)
       // dbus_bus_add_match is used for the notification mechanism,
       // and this works only for type DBUS_TYPE_STRING as message arguments
       // this is the reason to use string instead of integer types directly
-      snprintf(ldbidArray, DbusSubMatchSize, "%d", gSendNotifyLdbid);
-      snprintf(userArray,  DbusSubMatchSize, "%d", gSendNotifyUserNo);
-      snprintf(seatArray,  DbusSubMatchSize, "%d", gSendNotifySeatNo);
+      snprintf(ldbidArray, DbusSubMatchSize, "%u", gNotifyLdbid);
+      snprintf(userArray,  DbusSubMatchSize, "%u", gNotifyUserNo);
+      snprintf(seatArray,  DbusSubMatchSize, "%u", gNotifySeatNo);
 
       //printf("process_send_Notification_Signal => key: %s | lbid: %d | gUserNo: %d | gSeatNo: %d | gReason: %d \n", gNotifykey, gLdbid, gUserNo, gSeatNo, gReason);
       message = dbus_message_new_signal("/org/genivi/persistence/adminconsumer",    // const char *path,
@@ -374,20 +374,12 @@ void process_send_lifecycle_register(DBusConnection* conn, int regType, int shut
                                               DBUS_TYPE_UINT32, &shutdownMode, DBUS_TYPE_INVALID);
          }
 
-         if(conn != NULL)
-         {
-            if(!dbus_connection_send(conn, message, 0))
-            {
-               DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("send_lifecycle_register => Access denied"), DLT_STRING(error.message) );
-               rval = -1;
-            }
-            dbus_connection_flush(conn);
-         }
-         else
-         {
-            DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("send_lifecycle_register => ERROR: Invalid connection"));
-            rval = -1;
-         }
+                if(!dbus_connection_send(conn, message, 0))
+                {
+                   DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("send_lifecycle_register => Access denied"), DLT_STRING(error.message) );
+                   rval = -1;
+                }
+                dbus_connection_flush(conn);
          dbus_message_unref(message);
       }
       else
@@ -421,21 +413,14 @@ void process_send_lifecycle_request(DBusConnection* conn, int requestId, int sta
                                            DBUS_TYPE_INT32, &status,
                                            DBUS_TYPE_INVALID);
 
-         if(conn != NULL)
-         {
-            if(!dbus_connection_send(conn, message, 0))
-            {
-               DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("send_lifecycle_request => Access denied"), DLT_STRING(error.message) );
-               rval = -1;
-            }
 
-            dbus_connection_flush(conn);
-         }
-         else
-         {
-            DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("send_lifecycle_request => ERROR: Invalid connection"));
-            rval = -1;
-         }
+                if(!dbus_connection_send(conn, message, 0))
+                {
+                   DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("send_lifecycle_request => Access denied"), DLT_STRING(error.message) );
+                   rval = -1;
+                }
+
+                dbus_connection_flush(conn);
          dbus_message_unref(message);
       }
       else

diff --git a/src/persistence_client_library_dbus_service.c b/src/persistence_client_library_dbus_service.c
index aade4d3..86e53ae 100644 (file)
--- a/src/persistence_client_library_dbus_service.c
+++ b/src/persistence_client_library_dbus_service.c
@@ -30,8 +30,11 @@
 
 pthread_cond_t  gDbusInitializedCond = PTHREAD_COND_INITIALIZER;
 pthread_mutex_t gDbusInitializedMtx  = PTHREAD_MUTEX_INITIALIZER;
+
 pthread_mutex_t gDbusPendingRegMtx   = PTHREAD_MUTEX_INITIALIZER;
+
 pthread_mutex_t gMainLoopMtx         = PTHREAD_MUTEX_INITIALIZER;
+pthread_cond_t  gMainLoopCond = PTHREAD_COND_INITIALIZER;
 
 int gEfds;
 
@@ -274,6 +277,7 @@ int setup_dbus_mainloop(void)
          {
             DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("dbus_bus_register() Error :"), DLT_STRING(err.message) );
             dbus_error_free (&err);
+            pthread_mutex_unlock(&gDbusInitializedMtx);
             return -1;
          }
       }
@@ -281,6 +285,7 @@ int setup_dbus_mainloop(void)
       {
          DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("dbus_connection_open_private() Error :"), DLT_STRING(err.message) );
          dbus_error_free(&err);
+         pthread_mutex_unlock(&gDbusInitializedMtx);
          return -1;
       }
    }
@@ -296,6 +301,7 @@ int setup_dbus_mainloop(void)
    if(rval)
    {
      DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("pthread_create( DBUS run_mainloop ) returned an error:"), DLT_INT(rval) );
+     pthread_mutex_unlock(&gDbusInitializedMtx);
      return -1;
    }
 
@@ -559,12 +565,13 @@ int mainLoop(DBusObjectPathVTable vtable, DBusObjectPathVTable vtable2,
                                  uint16_t buf[64];
                                  bContinue = TRUE;
                                  while ((-1==(ret = read(gPollInfo.fds[i].fd, buf, 64)))&&(EINTR == errno));
-                                 if (0>ret)
+                                 if(ret < 0)
                                  {
                                     DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("mainLoop => read() failed"), DLT_STRING(strerror(errno)) );
                                  }
-                                 else if (ret != -1)
+                                 else
                                  {
+                                    pthread_mutex_lock(&gMainLoopMtx);
                                     switch (buf[0])
                                     {
                                        case CMD_PAS_BLOCK_AND_WRITE_BACK:
@@ -598,6 +605,7 @@ int mainLoop(DBusObjectPathVTable vtable, DBusObjectPathVTable vtable2,
                                           DLT_LOG(gDLTContext, DLT_LOG_ERROR, DLT_STRING("mainLoop => command not handled"), DLT_INT(buf[0]) );
                                           break;
                                     }
+                                    pthread_cond_signal(&gMainLoopCond);
                                     pthread_mutex_unlock(&gMainLoopMtx);
                                  }
                               }
@@ -664,6 +672,10 @@ int deliverToMainloop(tCmd mainloopCmd, unsigned int param1, unsigned int param2
      rval = -1;
    }
 
+   // wait for condition variable
+   pthread_cond_wait(&gMainLoopCond, &gMainLoopMtx);
+   pthread_mutex_unlock(&gMainLoopMtx);
+
    return rval;
 }
 

diff --git a/test/persistence_admin_service_mockup.c b/test/persistence_admin_service_mockup.c
index 0d1784a..21dc297 100644 (file)
--- a/test/persistence_admin_service_mockup.c
+++ b/test/persistence_admin_service_mockup.c
@@ -128,7 +128,7 @@ int checkAdminMsg(DBusConnection *connection, DBusMessage *message)
    }
 
 
-   printf("   checkAdminMsg ==> busName: %s | objName: %s | notificationFlag: %u | gTimeoutMs: %d\n\n", busName, objName, notificationFlag, gTimeoutMs);
+   printf("   checkAdminMsg ==> busName: %s | objName: %s | notificationFlag: %d | gTimeoutMs: %u\n\n", busName, objName, notificationFlag, gTimeoutMs);
    reply = dbus_message_new_method_return(message);
 
    if (reply == 0)

diff --git a/test/persistence_lifeCycle_mockup.c b/test/persistence_lifeCycle_mockup.c
index 8e0fd6a..13b6a7e 100644 (file)
--- a/test/persistence_lifeCycle_mockup.c
+++ b/test/persistence_lifeCycle_mockup.c
@@ -157,7 +157,7 @@ int checkAdminMsg(DBusConnection *connection, DBusMessage *message, int reg)
    }
 
 
-   printf("   checkAdminMsg ==> busName: %s | objName: %s | notificationFlag: %u | gTimeoutMs: %d\n\n", busName, objName, notificationFlag, gTimeoutMs);
+   printf("   checkAdminMsg ==> busName: %s | objName: %s | notificationFlag: %d | gTimeoutMs: %u\n\n", busName, objName, notificationFlag, gTimeoutMs);
    reply = dbus_message_new_method_return(message);
 
    if (reply == 0)