Fixed memory issues related to AddClientCB

AddClientCB had a weak memory-guarantee that caused 3 values to be
leaked in certain conditions.  This will prevent those from happening.

The result-setting is done in order to ensure that the values are always
properly free'd.

Change-Id: I027fab84ddf62526d51a6e7e35dd54a96acbb48d
Signed-off-by: Erich Keane <erich.keane@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/502
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Sachin Agrawal <sachin.agrawal@intel.com>
Reviewed-by: Sashi Penta <sashi.kumar.penta@intel.com>

diff --git a/resource/csdk/stack/include/internal/occlientcb.h b/resource/csdk/stack/include/internal/occlientcb.h
index b3f100f..5ee8995 100644 (file)
--- a/resource/csdk/stack/include/internal/occlientcb.h
+++ b/resource/csdk/stack/include/internal/occlientcb.h
@@ -140,7 +140,7 @@ ClientCB* GetClientCB(const CAToken_t * token, OCDoHandle handle, const char * r
  *      OC_STACK_NO_MEMORY when out of memory
  */
 #ifdef WITH_PRESENCE
-OCStackResult InsertResourceTypeFilter(ClientCB * cbNode, const char * resourceTypeName);
+OCStackResult InsertResourceTypeFilter(ClientCB * cbNode, char * resourceTypeName);
 #endif // WITH_PRESENCE
 
 /** @ingroup ocstack

diff --git a/resource/csdk/stack/src/occlientcb.c b/resource/csdk/stack/src/occlientcb.c
index d93c69d..6aa83ac 100644 (file)
--- a/resource/csdk/stack/src/occlientcb.c
+++ b/resource/csdk/stack/src/occlientcb.c
@@ -86,15 +86,18 @@ AddClientCB (ClientCB** clientCB, OCCallbackData* cbData,
         // Ensure that the handle the SDK hands back up to the application layer for the
         // OCDoResource call matches the found ClientCB Node.
         *clientCB = cbNode;
-        OCFree(requestUri);
+        cbData->cd(cbData->context);
+        OCFree(*token);
         OCFree(*handle);
+        OCFree(requestUri);
+        OCFree(resourceTypeName);
         *handle = cbNode->handle;
     }
 
     #ifdef WITH_PRESENCE
     if(method == OC_REST_PRESENCE && resourceTypeName)
     {   // Amend the found or created node by adding a new resourceType to it.
-        return InsertResourceTypeFilter(cbNode, (const char *)resourceTypeName);
+        return InsertResourceTypeFilter(cbNode, resourceTypeName);
     }
     #endif
 
@@ -175,7 +178,7 @@ ClientCB* GetClientCB(const CAToken_t * token, OCDoHandle handle, const char * r
 }
 
 #ifdef WITH_PRESENCE
-OCStackResult InsertResourceTypeFilter(ClientCB * cbNode, const char * resourceTypeName)
+OCStackResult InsertResourceTypeFilter(ClientCB * cbNode, char * resourceTypeName)
 {
     OCResourceType * newResourceType = NULL;
     if(cbNode && resourceTypeName)
@@ -188,7 +191,7 @@ OCStackResult InsertResourceTypeFilter(ClientCB * cbNode, const char * resourceT
         }
 
         newResourceType->next = NULL;
-        newResourceType->resourcetypename = (char *) resourceTypeName;
+        newResourceType->resourcetypename = resourceTypeName;
 
         LL_APPEND(cbNode->filterResourceType, newResourceType);
         return OC_STACK_OK;

diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c
index 6e2427f..a77f80a 100644 (file)
--- a/resource/csdk/stack/src/ocstack.c
+++ b/resource/csdk/stack/src/ocstack.c
@@ -1446,6 +1446,7 @@ OCStackResult OCDoResource(OCDoHandle *handle, OCMethod method, const char *requ
     {
         OC_LOG(ERROR, TAG, PCF("CAGenerateToken error"));
         CADestroyToken(token);
+        result = CAResultToOCResult(caResult);
         goto exit;
     }
 
@@ -1501,6 +1502,7 @@ OCStackResult OCDoResource(OCDoHandle *handle, OCMethod method, const char *requ
         if (caResult != CA_STATUS_OK)
         {
             OC_LOG(ERROR, TAG, PCF("CACreateRemoteEndpoint error"));
+            result = CAResultToOCResult(caResult);
             goto exit;
         }
 
@@ -1510,6 +1512,7 @@ OCStackResult OCDoResource(OCDoHandle *handle, OCMethod method, const char *requ
     if (caResult != CA_STATUS_OK)
     {
         OC_LOG(ERROR, TAG, PCF("CASendRequest"));
+        result = CAResultToOCResult(caResult);
         goto exit;
     }
 
@@ -1535,6 +1538,8 @@ exit:
         OC_LOG(ERROR, TAG, PCF("OCDoResource error"));
         FindAndDeleteClientCB(clientCB);
         OCFree(resHandle);
+        OCFree(requestUri);
+        OCFree(resourceType);
     }
     CADestroyRemoteEndpoint(endpoint);
     OCFree(grpEnd.resourceUri);

diff --git a/resource/unittests/OCResourceTest.cpp b/resource/unittests/OCResourceTest.cpp
index b2a0c84..be804f4 100644 (file)
--- a/resource/unittests/OCResourceTest.cpp
+++ b/resource/unittests/OCResourceTest.cpp
@@ -43,25 +43,25 @@ namespace OCResourceTest
     }
 
     //Helper method
-    OCResource::Ptr ConstructResourceObject(std::string uri)
+    OCResource::Ptr ConstructResourceObject(std::string host, std::string uri)
     {
         OCConnectivityType connectivityType = OC_WIFI;
         std::vector<std::string> types = {"intel.rpost"};
         std::vector<std::string> ifaces = {DEFAULT_INTERFACE};
 
-        return OCPlatform::constructResourceObject(std::string(""), uri,
+        return OCPlatform::constructResourceObject(host, uri,
                 connectivityType, false, types, ifaces);
     }
 
     //ConstructResourceTest
     TEST(ConstructResourceTest, ConstructResourceObject)
     {
-        EXPECT_ANY_THROW(ConstructResourceObject(std::string("")));
+        EXPECT_ANY_THROW(ConstructResourceObject(std::string(""), std::string("")));
     }
 
     TEST(ResourceGetTest, ResourceGetForValidUri)
     {
-        OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000");
+        OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000", "/resource");
         if(resource)
         {
             QueryParamsMap test;
@@ -69,9 +69,19 @@ namespace OCResourceTest
         }
     }
 
+    TEST(ResourceGetTest, ResourceGetForBadUri)
+    {
+        OCResource::Ptr resource = ConstructResourceObject("", "coap://192.168.1.2:5000");
+        if(resource)
+        {
+            QueryParamsMap test;
+            EXPECT_THROW(resource->get(OC::QueryParamsMap(), &onGetPut), OC::OCException);
+        }
+    }
+
     TEST(ResourcePutTest, ResourcePutForValid)
     {
-        OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000");
+        OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000", "/resource");
         if(resource)
         {
             QueryParamsMap test;
@@ -80,13 +90,12 @@ namespace OCResourceTest
         }
     }
 
-
     TEST(ResourcePostTest, ResourcePostValidConfiguration)
     {
         PlatformConfig cfg;
         OCPlatform::Configure(cfg);
 
-        OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000");
+        OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000", "/resource");
         if(resource)
         {
             OCRepresentation rep;
@@ -97,7 +106,7 @@ namespace OCResourceTest
 
     TEST(ResourceObserveTest, ResourceObserveValidUri)
     {
-        OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000");
+        OCResource::Ptr resource = ConstructResourceObject("coap://192.168.1.2:5000", "/resource");
         if(resource)
         {
             QueryParamsMap test;