Correct some defects detected by Static Code Analysis Tool in things manager
authorJihun Ha <jihun.ha@samsung.com>
Tue, 15 Dec 2015 00:53:48 +0000 (09:53 +0900)
committerUze Choi <uzchoi@samsung.com>
Mon, 21 Dec 2015 09:33:08 +0000 (09:33 +0000)
After running Static Code Analysis Tool(Svace) for things manager codes, some defects have been
detected. Most of the defects are about null pointer handling and variable
initialization. This patchset is for covering these defects.

Change-Id: I97c3a58f08576ef50b636e52027311e7e9e1d04c
Signed-off-by: Jihun Ha <jihun.ha@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/4465
Reviewed-by: Madan Lanka <lanka.madan@samsung.com>
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Uze Choi <uzchoi@samsung.com>
resource/csdk/stack/src/oicgroup.c
service/things-manager/sampleapp/linux/configuration/MaintenanceCollection.cpp [changed mode: 0644->0755]
service/things-manager/sampleapp/linux/configuration/con-client.cpp
service/things-manager/sampleapp/linux/groupaction/groupserver.cpp
service/things-manager/sdk/inc/ActionSet.h [changed mode: 0644->0755]
service/things-manager/sdk/src/ActionSet.cpp [changed mode: 0644->0755]
service/things-manager/sdk/src/GroupManager.cpp
service/things-manager/unittests/ThingsManagerTest.cpp

index d7eb9d9..6533a1d 100755 (executable)
@@ -653,6 +653,7 @@ OCStackResult BuildActionSetFromString(OCActionSet **set, char* actiondesc)
     VARIFY_POINTER_NULL(*set, result, exit)
 
     iterToken = (char *) strtok_r(actiondesc, ACTION_DELIMITER, &iterTokenPtr);
+    VARIFY_POINTER_NULL(iterToken, result, exit);
 
     // ActionSet Name
     memset(*set, 0, sizeof(OCActionSet));
@@ -704,6 +705,7 @@ OCStackResult BuildActionSetFromString(OCActionSet **set, char* actiondesc)
 
             attrIterToken = (char *) strtok_r(NULL, ATTR_ASSIGN,
                     &attrIterTokenPtr);
+            VARIFY_POINTER_NULL(attrIterToken, result, exit);
             value = (char *) OICMalloc(strlen(attrIterToken) + 1);
             VARIFY_POINTER_NULL(value, result, exit)
             VARIFY_PARAM_NULL(attrIterToken, result, exit)
@@ -785,14 +787,31 @@ OCStackResult BuildStringFromActionSet(OCActionSet* actionset, char** desc)
     char temp[1024] = { 0 };
     size_t remaining = sizeof(temp) - 1;
     OCStackResult res = OC_STACK_ERROR;
+    char* actionTypeStr = NULL;
 
     OCAction *action = actionset->head;
 
     if (remaining >= strlen(actionset->actionsetName) + 1)
     {
-        strcat(temp, actionset->actionsetName);
+        strncat(temp, actionset->actionsetName, strlen(actionset->actionsetName));
         remaining -= strlen(actionset->actionsetName);
-        strcat(temp, ACTION_DELIMITER);
+        strncat(temp, ACTION_DELIMITER, strlen(ACTION_DELIMITER));
+        remaining--;
+    }
+    else
+    {
+        res = OC_STACK_ERROR;
+        goto exit;
+    }
+
+    actionTypeStr = (char *)malloc(1024);
+    if(actionTypeStr != NULL)
+    {
+        sprintf(actionTypeStr, "%ld %u", actionset->timesteps, actionset->type);
+        strncat(temp, actionTypeStr, strlen(actionTypeStr));
+        remaining -= strlen(actionTypeStr);
+        free(actionTypeStr);
+        strncat(temp, ACTION_DELIMITER, strlen(ACTION_DELIMITER));
         remaining--;
     }
     else
@@ -946,7 +965,7 @@ OCStackResult BuildActionJSON(OCAction* action, unsigned char* bufferPtr,
     jsonLen = strlen(jsonStr);
     if (jsonLen < *remaining)
     {
-        strcat((char*) bufferPtr, jsonStr);
+        strncat((char*) bufferPtr, jsonStr, jsonLen);
         *remaining -= jsonLen;
         bufferPtr += jsonLen;
         ret = OC_STACK_OK;
old mode 100644 (file)
new mode 100755 (executable)
index 1b78dd8..d28ca48
@@ -111,7 +111,7 @@ void MaintenanceResource::maintenanceMonitor(int second)
             int res;
             std::cout << "Reboot will be soon..." << std::endl;
             m_reboot = defaultReboot;
-            res = system("sudo reboot"); // System reboot
+            res = system("/usr/bin/sudo /etc/init.d/reboot"); // System reboot for linux
 
             std::cout << "return: " << res << std::endl;
 
index b924733..d83e86d 100755 (executable)
@@ -220,7 +220,7 @@ void onFoundCandidateResource(std::vector< std::shared_ptr< OCResource > > resou
                 {
                     resourceTable[resource->host() + resource->uri()] = resource;
 
-                    OCResourceHandle foundResourceHandle;
+                    OCResourceHandle foundResourceHandle = NULL;
                     OCStackResult result = OCPlatform::registerResource(foundResourceHandle,
                             resource);
                     std::cout << "\tResource ( " << resource->host() << " ) is registed!\t"
@@ -334,6 +334,11 @@ int main()
                 std::cout << "Please put a digit, not string" << std::endl;
                 continue;
             }
+            catch(std::out_of_range&)
+            {
+                std::cout << "Please put a number within the supported range" << std::endl;
+                continue;
+            }
 
             if (g_Steps == 0)
             {
index 753c6f7..add3d12 100755 (executable)
@@ -36,10 +36,10 @@ namespace PH = std::placeholders;
 
 bool isReady = false;
 
-OCResourceHandle resourceHandle;
+OCResourceHandle resourceHandle = NULL;
 std::vector<OCResourceHandle> resourceHandleVector;
 
-shared_ptr<OCResource> g_resource;
+shared_ptr<OCResource> g_resource = NULL;
 vector<string> lights;
 
 GroupManager *groupMgr = new GroupManager();
@@ -64,37 +64,43 @@ shared_ptr<OCResource> g_light;
 void foundResources(
         std::vector<std::shared_ptr<OC::OCResource> > listOfResource)
 {
-
-    for (auto rsrc = listOfResource.begin(); rsrc != listOfResource.end();
-            ++rsrc)
+    try
     {
-        std::string resourceURI = (*rsrc)->uri();
-        std::string hostAddress = (*rsrc)->host();
-
-        if (resourceURI == "/a/light")
+        for (auto rsrc = listOfResource.begin(); rsrc != listOfResource.end();
+                ++rsrc)
         {
-            cout << "\tResource URI : " << resourceURI << endl;
-            cout << "\tResource Host : " << hostAddress << endl;
-
-            OCResourceHandle foundResourceHandle;
-            OCStackResult result = OCPlatform::registerResource(
-                    foundResourceHandle, (*rsrc));
-            cout << "\tresource registed!" << endl;
-            if (result == OC_STACK_OK)
-            {
-                OCPlatform::bindResource(resourceHandle, foundResourceHandle);
-                resourceHandleVector.push_back(foundResourceHandle);
-            }
-            else
+            std::string resourceURI = (*rsrc)->uri();
+            std::string hostAddress = (*rsrc)->host();
+
+            if (resourceURI == "/a/light")
             {
-                cout << "\tresource Error!" << endl;
-            }
+                cout << "\tResource URI : " << resourceURI << endl;
+                cout << "\tResource Host : " << hostAddress << endl;
+
+                OCResourceHandle foundResourceHandle = NULL;
+                OCStackResult result = OCPlatform::registerResource(
+                        foundResourceHandle, (*rsrc));
+                cout << "\tresource registed!" << endl;
+                if (result == OC_STACK_OK)
+                {
+                    OCPlatform::bindResource(resourceHandle, foundResourceHandle);
+                    resourceHandleVector.push_back(foundResourceHandle);
+                }
+                else
+                {
+                    cout << "\tresource Error!" << endl;
+                }
 
-            lights.push_back((hostAddress + resourceURI));
+                lights.push_back((hostAddress + resourceURI));
 
-            g_light = (*rsrc);
+                g_light = (*rsrc);
+            }
         }
     }
+    catch (std::exception& e)
+    {
+        std::cout << "Exception in foundResource:"<< e.what() << std::endl;
+    }
 
     isReady = true;
 }
old mode 100644 (file)
new mode 100755 (executable)
old mode 100644 (file)
new mode 100755 (executable)
index 910a5e6..e925ca9
@@ -23,7 +23,7 @@ using namespace std;
 
 namespace OIC
 {
-Time::Time()
+Time::Time() : mTime()
 {
     setTime(0, 0, 0, 0, 0, 0, 0);
 }
@@ -40,12 +40,9 @@ void Time::setTime(unsigned int yy, unsigned int mm, unsigned int dd,
         unsigned int h, unsigned int m, unsigned int s,
         int dayoftheweek = 0)
 {
-    yy -= 1900;
-    mm -= 1;
-
     mDelay = 0;
-    mTime.tm_year = yy;
-    mTime.tm_mon = mm;
+    mTime.tm_year = (int)yy - 1900;
+    mTime.tm_mon = (int)mm - 1;
     mTime.tm_mday = dd;
 
     mTime.tm_hour = h;
index 6249e31..8bbe52f 100755 (executable)
@@ -30,7 +30,6 @@
 
 #include "GroupManager.h"
 
-#define PLAIN_DELIMITER "\""
 #define ACTION_DELIMITER "*"
 #define DESC_DELIMITER "|"
 #define ATTR_DELIMITER "="
@@ -505,8 +504,10 @@ ActionSet* GroupManager::getActionSetfromString(std::string description)
     }
 
     plainText = new char[(description.length() + 1)];
-    strcpy(plainText, description.c_str());
+    strncpy(plainText, description.c_str(), description.length()+1);
 
+    // All tokens are seperated by "*" character.
+    // First token means an actionset name
     token = strtok_r(plainText, ACTION_DELIMITER, &plainPtr);
 
     if (token != NULL)
@@ -516,6 +517,7 @@ ActionSet* GroupManager::getActionSetfromString(std::string description)
         if((actionset->actionsetName).empty())
             goto exit;
 
+        // Find the second token
         token = strtok_r(NULL, ACTION_DELIMITER, &plainPtr);
     }
     else
@@ -525,8 +527,35 @@ ActionSet* GroupManager::getActionSetfromString(std::string description)
 
     if (token != NULL)
     {
-        sscanf(token, "%ld %d", &actionset->mDelay, (int*)&actionset->type);
+        // The second token consists of two digits: time delay and actionset Type.
+        // Time delay is used as a parameter to scheduled/recursive group action feature
+        // And actionset type indicates if the actionset is scheduled or recursive group action.
+        char *subText = NULL;
+        char *subToken = NULL;
+        char *subTextPtr = NULL;
 
+        subText = new char[strlen(token)+1];
+        strncpy(subText, token, strlen(token)+1);
+
+        subToken = strtok_r(subText, " ", &subTextPtr);
+        if(subToken == NULL)
+        {
+            DELETEARRAY(subText);
+            goto exit;
+        }
+        actionset->mDelay = atol(subToken);
+
+        subToken = strtok_r(NULL, " ", &subTextPtr);
+        if(subToken == NULL)
+        {
+            DELETEARRAY(subText);
+            goto exit;
+        }
+        actionset->type = (OIC::ACTIONSET_TYPE)atoi(subToken);
+
+        DELETEARRAY(subText);
+
+        // Find the third token
         token = strtok_r(NULL, ACTION_DELIMITER, &plainPtr);
     }
     else
@@ -536,25 +565,33 @@ ActionSet* GroupManager::getActionSetfromString(std::string description)
 
     while (token)
     {
+        // The third token consists of two sub-segement: host address plus URI and attribute key/value pair.
+        // These two segments can be divided by "|" character.
+        // The third token can be repeatively appeared.
+
         char *descPtr = NULL;
         desc = new char[(strlen(token) + 1)];
 
         if (desc != NULL)
         {
-            strcpy(desc, token);
+            // copy a host address plus URI string from the third token
+            strncpy(desc, token, (strlen(token) + 1));
+
+            // Find "|" character to find key/value pair
             token = strtok_r(desc, DESC_DELIMITER, &descPtr);
 
             while (token != NULL)
             {
                 char *attrPtr = NULL;
                 attr = new char[(strlen(token) + 1)];
-
+                // copy the host address plus URI string
                 strcpy(attr, token);
 
+                // Find "=" charactor to divide attribute key and value string.
                 token = strtok_r(attr, ATTR_DELIMITER, &attrPtr);
                 while (token != NULL)
                 {
-                    if ( (action == NULL) && strcmp(token, "uri") == 0)    //consider only first "uri" as uri, other as attribute.
+                    if ( (action == NULL) && strcmp(token, "uri") == 0)
                     {
                         token = strtok_r(NULL, ATTR_DELIMITER, &attrPtr);
                         if(token == NULL)
@@ -607,7 +644,6 @@ ActionSet* GroupManager::getActionSetfromString(std::string description)
         else
         {
             goto exit;
-
         }
 
         DELETEARRAY(desc);
index 2d018eb..980a075 100755 (executable)
@@ -558,7 +558,7 @@ void MaintenanceResource::maintenanceMonitor(int second)
             int res;
             std::cout << "Reboot will be soon..." << std::endl;
             m_reboot = defaultReboot;
-            res = system("sudo reboot");
+            res = system("/usr/bin/sudo /etc/init.d/reboot");
 
             std::cout << "return: " << res << std::endl;