Repaired a number of issues discovered while doing a code-review

I was poking through a general code review/testing out a SCA tool
and came up with/discovered a handful of potential bug-filled code.

In a few locations we were referring to a variable BEFORE testing for
validity which has some pretty nasty concequences.  I replaced usages
of strtok with strtok_r in a few places I came across it.  The IP
parsing code was a bit convoluted and unnecessary, so I made it more
clear.

Change-Id: If1157ca33d2fe176a4beac9b3e4b8a8090357751
Signed-off-by: Erich Keane <erich.keane@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/346
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Doug Hudson <douglas.hudson@intel.com>
Reviewed-by: Patrick Lankswert <patrick.lankswert@intel.com>

diff --git a/resource/csdk/stack/src/ocobserve.c b/resource/csdk/stack/src/ocobserve.c
index 6a859c1..2abc9b2 100644 (file)
--- a/resource/csdk/stack/src/ocobserve.c
+++ b/resource/csdk/stack/src/ocobserve.c
@@ -115,19 +115,22 @@ OCStackResult SendAllObserverNotification (OCMethod method, OCResource *resPtr,
                         resourceObserver->resUri, 0,
                         &(resourceObserver->addressInfo), resourceObserver->connectivityType);
 
-                request->observeResult = OC_STACK_OK;
-                if(request && result == OC_STACK_OK)
+                if(request)
                 {
-                    result = FormOCEntityHandlerRequest(&ehRequest, (OCRequestHandle) request,
-                                request->method, (OCResourceHandle) resPtr, request->query,
-                                request->reqJSONPayload, request->numRcvdVendorSpecificHeaderOptions,
-                                request->rcvdVendorSpecificHeaderOptions, OC_OBSERVE_NO_OPTION, 0);
+                    request->observeResult = OC_STACK_OK;
                     if(result == OC_STACK_OK)
                     {
-                        ehResult = resPtr->entityHandler(OC_REQUEST_FLAG, &ehRequest);
-                        if(ehResult == OC_EH_ERROR)
+                        result = FormOCEntityHandlerRequest(&ehRequest, (OCRequestHandle) request,
+                                    request->method, (OCResourceHandle) resPtr, request->query,
+                                    request->reqJSONPayload, request->numRcvdVendorSpecificHeaderOptions,
+                                    request->rcvdVendorSpecificHeaderOptions, OC_OBSERVE_NO_OPTION, 0);
+                        if(result == OC_STACK_OK)
                         {
-                            FindAndDeleteServerRequest(request);
+                            ehResult = resPtr->entityHandler(OC_REQUEST_FLAG, &ehRequest);
+                            if(ehResult == OC_EH_ERROR)
+                            {
+                                FindAndDeleteServerRequest(request);
+                            }
                         }
                     }
                 }
@@ -197,7 +200,6 @@ OCStackResult SendListObserverNotification (OCResource * resource,
     while(numIds)
     {
         OC_LOG_V(INFO, TAG, "Need to notify observation id %d", *obsIdList);
-        observation = NULL;
         observation = GetObserverUsingId (*obsIdList);
         if(observation)
         {
@@ -213,33 +215,36 @@ OCStackResult SendListObserverNotification (OCResource * resource,
                         observation->addr, observation->resUri, 0,
                         &(observation->addressInfo), observation->connectivityType);
 
-                request->observeResult = OC_STACK_OK;
-                if(request && result == OC_STACK_OK)
+                if(request)
                 {
-                    memset(&ehResponse, 0, sizeof(OCEntityHandlerResponse));
-                    ehResponse.ehResult = OC_EH_OK;
-                    ehResponse.payload = (unsigned char *) OCMalloc(MAX_RESPONSE_LENGTH);
-                    if(!ehResponse.payload)
+                    request->observeResult = OC_STACK_OK;
+                    if(result == OC_STACK_OK)
                     {
-                        FindAndDeleteServerRequest(request);
-                        continue;
+                        memset(&ehResponse, 0, sizeof(OCEntityHandlerResponse));
+                        ehResponse.ehResult = OC_EH_OK;
+                        ehResponse.payload = (unsigned char *) OCMalloc(MAX_RESPONSE_LENGTH);
+                        if(!ehResponse.payload)
+                        {
+                            FindAndDeleteServerRequest(request);
+                            continue;
+                        }
+                        strcpy((char *)ehResponse.payload, (const char *)notificationJSONPayload);
+                        ehResponse.payloadSize = strlen((const char *)ehResponse.payload) + 1;
+                        ehResponse.persistentBufferFlag = 0;
+                        ehResponse.requestHandle = (OCRequestHandle) request;
+                        ehResponse.resourceHandle = (OCResourceHandle) resource;
+                        result = OCDoResponse(&ehResponse);
+                        if(result == OC_STACK_OK)
+                        {
+                            OCFree(ehResponse.payload);
+                            FindAndDeleteServerRequest(request);
+                        }
                     }
-                    strcpy((char *)ehResponse.payload, (const char *)notificationJSONPayload);
-                    ehResponse.payloadSize = strlen((const char *)ehResponse.payload) + 1;
-                    ehResponse.persistentBufferFlag = 0;
-                    ehResponse.requestHandle = (OCRequestHandle) request;
-                    ehResponse.resourceHandle = (OCResourceHandle) resource;
-                    result = OCDoResponse(&ehResponse);
-                    if(result == OC_STACK_OK)
+                    else
                     {
-                        OCFree(ehResponse.payload);
                         FindAndDeleteServerRequest(request);
                     }
                 }
-                else
-                {
-                    FindAndDeleteServerRequest(request);
-                }
 
                 numSentNotification++;
             }

diff --git a/resource/csdk/stack/src/ocserverrequest.c b/resource/csdk/stack/src/ocserverrequest.c
index 38bda0c..482b784 100644 (file)
--- a/resource/csdk/stack/src/ocserverrequest.c
+++ b/resource/csdk/stack/src/ocserverrequest.c
@@ -385,12 +385,12 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse)
     }
 
     // Allocate memory for the payload.
-    char *payload = (char *)OCMalloc(MAX_RESPONSE_LENGTH);
+    char *payload = (char *)OCCalloc(1, MAX_RESPONSE_LENGTH);
     if(!payload)
     {
         return OC_STACK_NO_MEMORY;
     }
-    memset(payload, 0, MAX_RESPONSE_LENGTH);
+
     // Put the JSON prefix and suffix around the payload
     strcpy(payload, (const char *)OC_JSON_PREFIX);
     strcat(payload, (const char *)ehResponse->payload);

diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c
index aaef714..c91cac3 100644 (file)
--- a/resource/csdk/stack/src/ocstack.c
+++ b/resource/csdk/stack/src/ocstack.c
@@ -318,6 +318,7 @@ OCStackResult UpdateResponseAddr(OCClientResponse *response, const CARemoteEndpo
     OCStackResult ret = OC_STACK_ERROR;
     static OCDevAddr address = {0};
     char * tok = NULL;
+    char * savePtr = NULL;
     char * cpAddress = (char *) OCMalloc(strlen(endPoint->addressInfo.IP.ipAddress) + 1);
     if(!cpAddress)
     {
@@ -328,33 +329,18 @@ OCStackResult UpdateResponseAddr(OCClientResponse *response, const CARemoteEndpo
             strlen(endPoint->addressInfo.IP.ipAddress) + 1);
 
     // Grabs the first three numbers from the IPv4 address and replaces dots
-    for(int i=0; i<3; i++)
+    for(int i=0; i<4; i++)
     {
-        if(i==0)
-        {
-            tok = strtok(cpAddress, ".");
-        }
-        else
-        {
-            tok = strtok(NULL, ".");
-        }
+        tok = strtok_r(i==0 ? cpAddress : NULL, ".", &savePtr);
+
         if(!tok)
         {
             ret = OC_STACK_ERROR;
             goto exit;
         }
         address.addr[i] = atoi(tok);
-        cpAddress[strlen(cpAddress)]='.'; // Replaces the dot here.
     }
 
-    // Grabs the last number from the IPv4 address - has no dot to replace
-    tok = strtok(NULL, ".");
-    if(!tok)
-    {
-        ret = OC_STACK_ERROR;
-        goto exit;
-    }
-    address.addr[3] = atoi(tok);
     memcpy(&address.addr[4], &endPoint->addressInfo.IP.port, sizeof(uint32_t));
 
     if(response)
@@ -374,19 +360,19 @@ exit:
 void parsePresencePayload(char* payload, uint32_t* seqNum, uint32_t* maxAge, char** resType)
 {
     char * tok = NULL;
-
+    char * savePtr;
     // The format of the payload is {"oc":[%u:%u:%s]}
     // %u : sequence number,
     // %u : max age
     // %s : Resource Type (Optional)
-    tok = strtok(payload, "[:]}");
+    tok = strtok_r(payload, "[:]}", &savePtr);
     payload[strlen(payload)] = ':';
-    tok = strtok(NULL, "[:]}");
+    tok = strtok_r(NULL, "[:]}", &savePtr);
     payload[strlen((char *)payload)] = ':';
     *seqNum = (uint32_t) atoi(tok);
-    tok = strtok(NULL, "[:]}");
+    tok = strtok_r(NULL, "[:]}", &savePtr);
     *maxAge = (uint32_t) atoi(tok);
-    tok = strtok(NULL, "[:]}");
+    tok = strtok_r(NULL, "[:]}",&savePtr);
 
     if(tok)
     {
@@ -956,16 +942,17 @@ OCStackResult HandleStackResponses(OCResponse * response)
             result = OC_STACK_INVALID_PARAM;
             goto exit;
         }
-        tok = strtok((char *)bufRes, "[:]}");
+        char * savePtr;
+        tok = strtok_r((char *)bufRes, "[:]}", &savePtr);
         bufRes[strlen((char *)bufRes)] = ':';
-        tok = strtok(NULL, "[:]}");
+        tok = strtok_r(NULL, "[:]}", &savePtr);
         bufRes[strlen((char *)bufRes)] = ':';
         response->clientResponse->sequenceNumber = (uint32_t )atoi(tok);
         OC_LOG_V(DEBUG, TAG, "The received NONCE is %u", response->clientResponse->sequenceNumber);
-        tok = strtok(NULL, "[:]}");
+        tok = strtok_r(NULL, "[:]}", &savePtr);
         response->maxAge = (uint32_t )atoi(tok);
         OC_LOG_V(DEBUG, TAG, "The received TTL is %u", response->maxAge);
-        tok = strtok(NULL, "[:]}");
+        tok = strtok_r(NULL, "[:]}", &savePtr);
         if(tok)
         {
             resourceTypeName = (char *)OCMalloc(strlen(tok));

diff --git a/resource/src/InProcServerWrapper.cpp b/resource/src/InProcServerWrapper.cpp
index d60f613..9287106 100644 (file)
--- a/resource/src/InProcServerWrapper.cpp
+++ b/resource/src/InProcServerWrapper.cpp
@@ -54,8 +54,11 @@ void formResourceRequest(OCEntityHandlerFlag flag,
                          OCEntityHandlerRequest * entityHandlerRequest,
                          std::shared_ptr<OCResourceRequest> pRequest)
 {
-    pRequest->setRequestHandle(entityHandlerRequest->requestHandle);
-    pRequest->setResourceHandle(entityHandlerRequest->resource);
+    if(pRequest && entityHandlerRequest)
+    {
+        pRequest->setRequestHandle(entityHandlerRequest->requestHandle);
+        pRequest->setResourceHandle(entityHandlerRequest->resource);
+    }
 
     if(flag & OC_INIT_FLAG)
     {