From 886e1580663affb94b7d0d946ccf3d0125cc3cf9 Mon Sep 17 00:00:00 2001 From: Jihun Ha Date: Tue, 15 Dec 2015 09:53:48 +0900 Subject: [PATCH] Correct some defects detected by Static Code Analysis Tool in things manager 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 Reviewed-on: https://gerrit.iotivity.org/gerrit/4465 Reviewed-by: Madan Lanka Tested-by: jenkins-iotivity Reviewed-by: Uze Choi --- resource/csdk/stack/src/oicgroup.c | 25 ++++++++-- .../linux/configuration/MaintenanceCollection.cpp | 2 +- .../sampleapp/linux/configuration/con-client.cpp | 7 ++- .../sampleapp/linux/groupaction/groupserver.cpp | 58 ++++++++++++---------- service/things-manager/sdk/inc/ActionSet.h | 0 service/things-manager/sdk/src/ActionSet.cpp | 9 ++-- service/things-manager/sdk/src/GroupManager.cpp | 50 ++++++++++++++++--- .../things-manager/unittests/ThingsManagerTest.cpp | 2 +- 8 files changed, 108 insertions(+), 45 deletions(-) mode change 100644 => 100755 service/things-manager/sampleapp/linux/configuration/MaintenanceCollection.cpp mode change 100644 => 100755 service/things-manager/sdk/inc/ActionSet.h mode change 100644 => 100755 service/things-manager/sdk/src/ActionSet.cpp diff --git a/resource/csdk/stack/src/oicgroup.c b/resource/csdk/stack/src/oicgroup.c index d7eb9d9..6533a1d 100755 --- a/resource/csdk/stack/src/oicgroup.c +++ b/resource/csdk/stack/src/oicgroup.c @@ -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; diff --git a/service/things-manager/sampleapp/linux/configuration/MaintenanceCollection.cpp b/service/things-manager/sampleapp/linux/configuration/MaintenanceCollection.cpp old mode 100644 new mode 100755 index 1b78dd8..d28ca48 --- a/service/things-manager/sampleapp/linux/configuration/MaintenanceCollection.cpp +++ b/service/things-manager/sampleapp/linux/configuration/MaintenanceCollection.cpp @@ -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; diff --git a/service/things-manager/sampleapp/linux/configuration/con-client.cpp b/service/things-manager/sampleapp/linux/configuration/con-client.cpp index b924733..d83e86d 100755 --- a/service/things-manager/sampleapp/linux/configuration/con-client.cpp +++ b/service/things-manager/sampleapp/linux/configuration/con-client.cpp @@ -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) { diff --git a/service/things-manager/sampleapp/linux/groupaction/groupserver.cpp b/service/things-manager/sampleapp/linux/groupaction/groupserver.cpp index 753c6f7..add3d12 100755 --- a/service/things-manager/sampleapp/linux/groupaction/groupserver.cpp +++ b/service/things-manager/sampleapp/linux/groupaction/groupserver.cpp @@ -36,10 +36,10 @@ namespace PH = std::placeholders; bool isReady = false; -OCResourceHandle resourceHandle; +OCResourceHandle resourceHandle = NULL; std::vector resourceHandleVector; -shared_ptr g_resource; +shared_ptr g_resource = NULL; vector lights; GroupManager *groupMgr = new GroupManager(); @@ -64,37 +64,43 @@ shared_ptr g_light; void foundResources( std::vector > 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; } diff --git a/service/things-manager/sdk/inc/ActionSet.h b/service/things-manager/sdk/inc/ActionSet.h old mode 100644 new mode 100755 diff --git a/service/things-manager/sdk/src/ActionSet.cpp b/service/things-manager/sdk/src/ActionSet.cpp old mode 100644 new mode 100755 index 910a5e6..e925ca9 --- a/service/things-manager/sdk/src/ActionSet.cpp +++ b/service/things-manager/sdk/src/ActionSet.cpp @@ -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; diff --git a/service/things-manager/sdk/src/GroupManager.cpp b/service/things-manager/sdk/src/GroupManager.cpp index 6249e31..8bbe52f 100755 --- a/service/things-manager/sdk/src/GroupManager.cpp +++ b/service/things-manager/sdk/src/GroupManager.cpp @@ -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); diff --git a/service/things-manager/unittests/ThingsManagerTest.cpp b/service/things-manager/unittests/ThingsManagerTest.cpp index 2d018eb..980a075 100755 --- a/service/things-manager/unittests/ThingsManagerTest.cpp +++ b/service/things-manager/unittests/ThingsManagerTest.cpp @@ -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; -- 2.7.4