Fixed memory handling of HandleSingleResponse

As identified in the RI code review, the options and token
were leaked and had some interesting behavior if the options count
was zero.  This patch corrects those, and repairs some return
values that were inconsistent and detected while chaising the
memory around.

Change-Id: Ica590fefc8702faa7108cc00642cbc2424f402ac
Signed-off-by: Erich Keane <erich.keane@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/564
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Joseph Morrow <joseph.l.morrow@intel.com>

diff --git a/resource/csdk/stack/src/ocserverrequest.c b/resource/csdk/stack/src/ocserverrequest.c
index 6c74095..30653d5 100644 (file)
--- a/resource/csdk/stack/src/ocserverrequest.c
+++ b/resource/csdk/stack/src/ocserverrequest.c
@@ -291,7 +291,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse)
     OCStackResult result = OC_STACK_ERROR;
     CARemoteEndpoint_t responseEndpoint = {};
     CAResponseInfo_t responseInfo = {};
-    CAHeaderOption_t* optionsPointer;
+    CAHeaderOption_t* optionsPointer = NULL;
 
     OC_LOG_V(INFO, TAG, "Inside HandleSingleResponse: %s", ehResponse->payload);
 
@@ -328,12 +328,8 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse)
             break;
     }
     responseInfo.info.type = qualityOfServiceToMessageType(serverRequest->qos);
-    responseInfo.info.token = (CAToken_t) OCCalloc(1, CA_MAX_TOKEN_LEN + 1);
-    if (!responseInfo.info.token)
-    {
-        OC_LOG(FATAL, TAG, "Response Info Token is NULL");
-        return result;
-    }
+    char token[CA_MAX_TOKEN_LEN + 1] = {};
+    responseInfo.info.token = token;
     memcpy(responseInfo.info.token, serverRequest->requestToken, CA_MAX_TOKEN_LEN);
 
     if(serverRequest->observeResult == OC_STACK_OK)
@@ -345,38 +341,48 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse)
         responseInfo.info.numOptions = ehResponse->numSendVendorSpecificHeaderOptions;
     }
 
-    responseInfo.info.options = (CAHeaderOption_t *)
-                                  OCMalloc(sizeof(CAHeaderOption_t) * responseInfo.info.numOptions);
+    if(responseInfo.info.numOptions > 0)
+    {
+        responseInfo.info.options = (CAHeaderOption_t *)
+                                      OCCalloc(responseInfo.info.numOptions,
+                                              sizeof(CAHeaderOption_t));
 
-    optionsPointer = responseInfo.info.options;
+        if(!responseInfo.info.options)
+        {
+            OC_LOG(FATAL, TAG, PCF("options is NULL"));
+            return OC_STACK_NO_MEMORY;
+        }
 
-    // TODO: This exposes CoAP specific details.  At some point, this should be
-    // re-factored and handled in the CA layer.
-    if(serverRequest->observeResult == OC_STACK_OK)
-    {
-        responseInfo.info.options[0].protocolID = CA_COAP_ID;
-        responseInfo.info.options[0].optionID = COAP_OPTION_OBSERVE;
-        responseInfo.info.options[0].optionLength = sizeof(uint32_t);
-        memcpy(responseInfo.info.options[0].optionData,
-                &(serverRequest->observationOption), sizeof(uint32_t));
-
-        // Point to the next header option before copying vender specific header options
-        optionsPointer += 1;
-    }
+        optionsPointer = responseInfo.info.options;
 
-    if (ehResponse->numSendVendorSpecificHeaderOptions)
-    {
-        memcpy(optionsPointer, ehResponse->sendVendorSpecificHeaderOptions,
-                        sizeof(OCHeaderOption) * ehResponse->numSendVendorSpecificHeaderOptions);
-    }
+        // TODO: This exposes CoAP specific details.  At some point, this should be
+        // re-factored and handled in the CA layer.
+        if(serverRequest->observeResult == OC_STACK_OK)
+        {
+            responseInfo.info.options[0].protocolID = CA_COAP_ID;
+            responseInfo.info.options[0].optionID = COAP_OPTION_OBSERVE;
+            responseInfo.info.options[0].optionLength = sizeof(uint32_t);
+            memcpy(responseInfo.info.options[0].optionData,
+                    &(serverRequest->observationOption), sizeof(uint32_t));
+
+            // Point to the next header option before copying vender specific header options
+            optionsPointer += 1;
+        }
 
-    // Allocate memory for the payload.
-    char *payload = (char *)OCCalloc(1, MAX_RESPONSE_LENGTH);
-    if(!payload)
+        if (ehResponse->numSendVendorSpecificHeaderOptions)
+        {
+            memcpy(optionsPointer, ehResponse->sendVendorSpecificHeaderOptions,
+                            sizeof(OCHeaderOption) *
+                            ehResponse->numSendVendorSpecificHeaderOptions);
+        }
+    }
+    else
     {
-        return OC_STACK_NO_MEMORY;
+        responseInfo.info.options = NULL;
     }
 
+    char payload[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);
@@ -404,7 +410,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse)
             if(caResult != CA_STATUS_OK)
             {
                 OC_LOG_V(ERROR, TAG, "CASendResponse failed on %s", connTypes[i]);
-                result = OC_STACK_ERROR;
+                result = CAResultToOCResult(caResult);
             }
             else
             {
@@ -417,6 +423,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse)
     if(caResult != CA_STATUS_OK)
     {
         OC_LOG(ERROR, TAG, PCF("CASendResponse failed"));
+        result = CAResultToOCResult(caResult);
     }
     else
     {
@@ -424,7 +431,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse)
     }
     #endif
 
-    OCFree(payload);
+    OCFree(responseInfo.info.options);
     //Delete the request
     FindAndDeleteServerRequest(serverRequest);
     return result;