Provided a mechanism for deleting the context pointer when the C Stack deletes callba...

Fixed some line endings.

Fixed some line lengths.

Fixed a series of memory leaks in C++ callbacks which assumed the C stack owned/deleted OCCallbackData

Patch 2: Removed deletion of function pointers becaues GCC says this violates the C spec.
Patch 3: Comments from Joey regarding the comment on the OCClientContextDeleter typedef
Patch 4: changed deleter to deleteCallback per sashi's suggestion
Change-Id: I0fb71896afac93356c1bd2f4916eb9f3eaa0ad13

diff --git a/csdk/stack/include/internal/occlientcb.h b/csdk/stack/include/internal/occlientcb.h
index 17ccada..105e8b4 100644 (file)
--- a/csdk/stack/include/internal/occlientcb.h
+++ b/csdk/stack/include/internal/occlientcb.h
@@ -37,11 +37,14 @@ typedef struct ClientCB {
     OCClientResponseHandler callBack;
     // callback context data
     void * context;
+    // callback method to delete context data
+    OCClientContextDeleter deleteCallback;
     //  when a response is recvd with this token, above callback will be invoked
     OCCoAPToken * token;
     // Invocation handle tied to original call to OCDoResource()
     OCDoHandle handle;
-    // This is used to determine if all responses should be consumed or not. (For now, only pertains to OC_REST_OBSERVE_ALL Vs. OC_REST_OBSERVE functionality)
+    // This is used to determine if all responses should be consumed or not.
+    // (For now, only pertains to OC_REST_OBSERVE_ALL Vs. OC_REST_OBSERVE functionality)
     OCMethod method;
     // This is the sequence identifier the server applies to the invocation tied to 'handle'.
     uint32_t sequenceNumber;

diff --git a/csdk/stack/include/ocstack.h b/csdk/stack/include/ocstack.h
index 92d7e3e..c0b2605 100644 (file)
--- a/csdk/stack/include/ocstack.h
+++ b/csdk/stack/include/ocstack.h
@@ -196,8 +196,14 @@ typedef enum {
 /**
  * Client applications implement this callback to consume responses received from Servers.
  */
-typedef OCStackApplicationResult (* OCClientResponseHandler)(void *context, OCDoHandle handle, OCClientResponse * clientResponse);
+typedef OCStackApplicationResult (* OCClientResponseHandler)(void *context, OCDoHandle handle,
+    OCClientResponse * clientResponse);
 
+/**
+ * Client applications using a context pointer implement this callback to delete the
+ * context upon removal of the callback/context pointer from the internal callback-list
+ */
+typedef void (* OCClientContextDeleter)(void *context);
 
 /*
  * This info is passed from application to OC Stack when initiating a request to Server
@@ -205,6 +211,7 @@ typedef OCStackApplicationResult (* OCClientResponseHandler)(void *context, OCDo
 typedef struct {
     void *context;
     OCClientResponseHandler cb;
+    OCClientContextDeleter cd;
 } OCCallbackData;
 
 /**

diff --git a/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp b/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp
index aca0d50..dd755c8 100644 (file)
--- a/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp
+++ b/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp
@@ -123,7 +123,7 @@ OCStackResult InvokeOCDoResource(std::ostringstream &query,
 
     cbData.cb = cb;
     cbData.context = (void*)CTX_VAL;
-
+    cbData.cd = NULL;
     ret = OCDoResource(&handle, method, query.str().c_str(), 0,
                        (method == OC_REST_PUT) ? putPayload.c_str() : NULL,
                        qos, &cbData);
@@ -346,6 +346,7 @@ int InitDiscovery()
     }
     cbData.cb = discoveryReqCB;
     cbData.context = (void*)CTX_VAL;
+    cbData.cd = NULL;
     ret = OCDoResource(&handle, OC_REST_GET, szQueryUri, 0, 0, OC_NON_CONFIRMABLE, &cbData);
     if (ret != OC_STACK_OK)
     {

diff --git a/csdk/stack/samples/linux/SimpleClientServer/occlientcoll.cpp b/csdk/stack/samples/linux/SimpleClientServer/occlientcoll.cpp
index 44713c3..c691df4 100644 (file)
--- a/csdk/stack/samples/linux/SimpleClientServer/occlientcoll.cpp
+++ b/csdk/stack/samples/linux/SimpleClientServer/occlientcoll.cpp
@@ -151,6 +151,7 @@ int InitGetRequestToUnavailableResource(OCClientResponse * clientResponse)
     getQuery << "coap://" << getIPAddrTBServer(clientResponse) << ":" << getPortTBServer(clientResponse) << "/SomeUnknownResource";
     cbData.cb = getReqCB;
     cbData.context = (void*)CTX_VAL;
+    cbData.cd = NULL;
     ret = OCDoResource(&handle, OC_REST_GET, getQuery.str().c_str(), 0, 0, OC_NON_CONFIRMABLE, &cbData);
     if (ret != OC_STACK_OK)
     {
@@ -169,6 +170,7 @@ int InitObserveRequest(OCClientResponse * clientResponse)
     obsReg << "coap://" << getIPAddrTBServer(clientResponse) << ":" << getPortTBServer(clientResponse) << getQueryStrForGetPut(clientResponse->resJSONPayload);
     cbData.cb = getReqCB;
     cbData.context = (void*)CTX_VAL;
+    cbData.cd = NULL;
     OC_LOG_V(INFO, TAG, "PUT payload from client = %s ", putPayload.c_str());
     ret = OCDoResource(&handle, OC_REST_OBSERVE, obsReg.str().c_str(), 0, 0, OC_NON_CONFIRMABLE, &cbData);
     if (ret != OC_STACK_OK)
@@ -194,6 +196,7 @@ int InitPutRequest(OCClientResponse * clientResponse)
     "/a/sroom?if=oc.mi.b";
     cbData.cb = putReqCB;
     cbData.context = (void*)CTX_VAL;
+    cbData.cd = NULL;
     OC_LOG_V(INFO, TAG, "PUT payload from client = %s ", putPayload.c_str());
     ret = OCDoResource(&handle, OC_REST_PUT, getQuery.str().c_str(), 0, putPayload.c_str(), OC_NON_CONFIRMABLE, &cbData);
     if (ret != OC_STACK_OK)
@@ -228,6 +231,7 @@ int InitGetRequest(OCClientResponse * clientResponse)
 
     cbData.cb = getReqCB;
     cbData.context = (void*)CTX_VAL;
+    cbData.cd = NULL;
     ret = OCDoResource(&handle, OC_REST_GET, getQuery.str().c_str(), 0, 0, OC_NON_CONFIRMABLE, &cbData);
     if (ret != OC_STACK_OK)
     {
@@ -250,6 +254,7 @@ int InitDiscovery()
 
     cbData.cb = discoveryReqCB;
     cbData.context = (void*)CTX_VAL;
+    cbData.cd = NULL;
     ret = OCDoResource(&handle, OC_REST_GET, szQueryUri, 0, 0, OC_NON_CONFIRMABLE, &cbData);
     if (ret != OC_STACK_OK)
     {

diff --git a/csdk/stack/src/occlientcb.c b/csdk/stack/src/occlientcb.c
index 9843513..b737223 100644 (file)
--- a/csdk/stack/src/occlientcb.c
+++ b/csdk/stack/src/occlientcb.c
@@ -38,6 +38,7 @@ OCStackResult AddClientCB(ClientCB** clientCB, OCCallbackData* cbData,
     if (cbNode) {
         cbNode->callBack = cbData->cb;
         cbNode->context = cbData->context;
+        cbNode->deleteCallback = cbData->cd;
         cbNode->token = token;
         cbNode->handle = handle;
         cbNode->method = method;
@@ -60,6 +61,11 @@ void DeleteClientCB(ClientCB * cbNode) {
         OCFree(cbNode->token);
         OCFree(cbNode->handle);
         OCFree(cbNode->requestUri);
+        if(cbNode->deleteCallback)
+        {
+            cbNode->deleteCallback(cbNode->context);
+        }
+
         #ifdef WITH_PRESENCE
         if(cbNode->presence) {
             OCFree(cbNode->presence->timeOut);
@@ -75,7 +81,8 @@ ClientCB* GetClientCB(OCCoAPToken * token, OCDoHandle * handle, unsigned char *
     ClientCB* out = NULL;
     if(token) {
         LL_FOREACH(cbList, out) {
-            if((out->token->tokenLength == token->tokenLength) && (memcmp(out->token->token, token->token, token->tokenLength) == 0) ) {
+            if((out->token->tokenLength == token->tokenLength) &&
+                (memcmp(out->token->token, token->token, token->tokenLength) == 0) ) {
                 return out;
             }
         }

diff --git a/csdk/stack/test/stacktests.cpp b/csdk/stack/test/stacktests.cpp
index 6778880..e8a303c 100644 (file)
--- a/csdk/stack/test/stacktests.cpp
+++ b/csdk/stack/test/stacktests.cpp
@@ -129,6 +129,7 @@ TEST(StackTest, DoResourceDeviceDiscovery) {
     strcpy(szQueryUri, OC_WELL_KNOWN_QUERY);
     cbData.cb = asyncDoResourcesCallback;
     cbData.context = (void*)CTX_VAL;
+    cbData.cd = NULL;
     EXPECT_EQ(OC_STACK_OK, OCDoResource(&handle, OC_REST_GET, szQueryUri, 0, 0, OC_NON_CONFIRMABLE, &cbData));
     //EXPECT_EQ(OC_STACK_OK, OCUpdateResources(SERVICE_URI));
     EXPECT_EQ(OC_STACK_OK, OCStop());
@@ -158,6 +159,7 @@ TEST(StackTest, UpdateResourceNullURI) {
     strcpy(szQueryUri, OC_WELL_KNOWN_QUERY);
     cbData.cb = asyncDoResourcesCallback;
     cbData.context = (void*)CTX_VAL;
+    cbData.cd = NULL;
     EXPECT_EQ(OC_STACK_OK, OCDoResource(&handle, OC_REST_GET, szQueryUri, 0, 0, OC_NON_CONFIRMABLE, &cbData));
     //EXPECT_EQ(OC_STACK_INVALID_URI, OCUpdateResources(0));
     EXPECT_EQ(OC_STACK_OK, OCStop());

diff --git a/src/InProcClientWrapper.cpp b/src/InProcClientWrapper.cpp
index 29bb7d1..b7b38e2 100644 (file)
--- a/src/InProcClientWrapper.cpp
+++ b/src/InProcClientWrapper.cpp
@@ -18,6 +18,7 @@
 //
 //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 
+#include <new>
 
 #include "InProcClientWrapper.h"
 #include "ocstack.h"
@@ -217,14 +218,15 @@ namespace OC
     {
         OCStackResult result;
 
-        OCCallbackData* cbdata = new OCCallbackData();
+        OCCallbackData cbdata = {0};
 
         ListenContext* context = new ListenContext();
         context->callback = callback;
         context->clientWrapper = shared_from_this();
 
-        cbdata->context =  static_cast<void*>(context);
-        cbdata->cb = listenCallback;
+        cbdata.context =  static_cast<void*>(context);
+        cbdata.cb = listenCallback;
+        cbdata.cd = [](void* c){delete static_cast<ListenContext*>(c);};
 
         auto cLock = m_csdkLock.lock();
         if(cLock)
@@ -235,7 +237,7 @@ namespace OC
                                   resourceType.c_str(),
                                   nullptr, nullptr,
                                   static_cast<OCQualityOfService>(m_cfg.QoS),
-                                  cbdata);
+                                  &cbdata);
         }
         else
         {
@@ -387,11 +389,13 @@ namespace OC
         const std::string& uri, const QueryParamsMap& queryParams, GetCallback& callback)
     {
         OCStackResult result;
-        OCCallbackData* cbdata = new OCCallbackData();
+        OCCallbackData cbdata = {0};
+
         GetContext* ctx = new GetContext();
         ctx->callback = callback;
-        cbdata->context = static_cast<void*>(ctx);
-        cbdata->cb = &getResourceCallback;
+        cbdata.context = static_cast<void*>(ctx);
+        cbdata.cb = &getResourceCallback;
+        cbdata.cd = [](void* c){delete static_cast<GetContext*>(c);};
 
         // TODO: in the future the cstack should be combining these two strings!
         ostringstream os;
@@ -407,7 +411,7 @@ namespace OC
             result = OCDoResource(&handle, OC_REST_GET, os.str().c_str(),
                                   nullptr, nullptr,
                                   static_cast<OCQualityOfService>(m_cfg.QoS),
-                                  cbdata);
+                                  &cbdata);
         }
         else
         {
@@ -487,17 +491,19 @@ namespace OC
         const QueryParamsMap& queryParams, PutCallback& callback)
     {
         OCStackResult result;
-        OCCallbackData* cbdata = new OCCallbackData();
+        OCCallbackData cbdata = {0};
+
         SetContext* ctx = new SetContext();
         ctx->callback = callback;
-        cbdata->cb = &setResourceCallback;
+        cbdata.cb = &setResourceCallback;
+        cbdata.cd = [](void* c){delete static_cast<SetContext*>(c);};
+        cbdata.context = static_cast<void*>(ctx);
 
         // TODO: in the future the cstack should be combining these two strings!
         ostringstream os;
         os << host << assembleSetResourceUri(uri, queryParams).c_str();
         // TODO: end of above
 
-        cbdata->context = static_cast<void*>(ctx);
         auto cLock = m_csdkLock.lock();
 
         if(cLock)
@@ -508,7 +514,7 @@ namespace OC
                                   os.str().c_str(), nullptr,
                                   assembleSetResourcePayload(attributes).c_str(),
                                   static_cast<OCQualityOfService>(m_cfg.QoS),
-                                  cbdata);
+                                  &cbdata);
         }
         else
         {
@@ -543,11 +549,13 @@ namespace OC
         ObserveCallback& callback)
     {
         OCStackResult result;
-        OCCallbackData* cbdata = new OCCallbackData();
+        OCCallbackData cbdata = {0};
+
         ObserveContext* ctx = new ObserveContext();
         ctx->callback = callback;
-        cbdata->context = static_cast<void*>(ctx);
-        cbdata->cb = &observeResourceCallback;
+        cbdata.context = static_cast<void*>(ctx);
+        cbdata.cb = &observeResourceCallback;
+        cbdata.cd = [](void* c){delete static_cast<ObserveContext*>(c);};
 
         OCMethod method;
         if (observeType == ObserveType::Observe)
@@ -577,7 +585,7 @@ namespace OC
                                   os.str().c_str(), nullptr,
                                   nullptr,
                                   static_cast<OCQualityOfService>(m_cfg.QoS),
-                                  cbdata);
+                                  &cbdata);
         }
         else
         {
@@ -625,12 +633,13 @@ namespace OC
         const std::string& host, SubscribeCallback& presenceHandler)
     {
         OCStackResult result;
-        OCCallbackData* cbdata = new OCCallbackData();
+        OCCallbackData cbdata = {0};
+
         SubscribePresenceContext* ctx = new SubscribePresenceContext();
         ctx->callback = presenceHandler;
-        cbdata->cb = &subscribePresenceCallback;
-        cbdata->context = static_cast<void*>(ctx);
-
+        cbdata.cb = &subscribePresenceCallback;
+        cbdata.context = static_cast<void*>(ctx);
+        cbdata.cd = [](void* c){delete static_cast<SubscribePresenceContext*>(c);};
         auto cLock = m_csdkLock.lock();
 
         std::ostringstream os;
@@ -642,7 +651,7 @@ namespace OC
         if(cLock)
         {
             result = OCDoResource(handle, OC_REST_PRESENCE, os.str().c_str(), nullptr, nullptr,
-                        OC_NON_CONFIRMABLE, cbdata);
+                        OC_NON_CONFIRMABLE, &cbdata);
         }
         else
         {