Fix result of notification valgrind memory checker.

When valgrind of notification service(including cpp wrapper) is run,
several leak was detected. (memory leak and invalid use of memory)

in this patch, result of valgrind was fixed.
1. invalid use of string in the unit test code, was fixed.
2. when returnning with error, allocated memory was freed.
3. unfreed variable was freed.
4. unit test build warning was fixed.

Change-Id: I4c8367bfda86813bb1ce1c71cf7841067c489ccc
Signed-off-by: KIM JungYong <jyong2.kim@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/19009
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Uze Choi <uzchoi@samsung.com>

diff --git a/service/notification/cpp-wrapper/provider/src/NSConsumer.cpp b/service/notification/cpp-wrapper/provider/src/NSConsumer.cpp
index 5d3c472..1ddca55 100755 (executable)
--- a/service/notification/cpp-wrapper/provider/src/NSConsumer.cpp
+++ b/service/notification/cpp-wrapper/provider/src/NSConsumer.cpp
@@ -32,6 +32,26 @@ namespace OIC
 {\r
     namespace Service\r
     {\r
+        namespace\r
+        {\r
+            void removeTopicLL(NSTopicLL * topicHead)\r
+            {\r
+                NSTopicLL * iter = topicHead;\r
+                NSTopicLL * following = NULL;\r
+\r
+                while (iter)\r
+                {\r
+                    following = iter->next;\r
+\r
+                    OICFree(iter->topicName);\r
+                    iter->next = NULL;\r
+                    OICFree(iter);\r
+\r
+                    iter = following;\r
+                }\r
+            }\r
+        }\r
+\r
         ::NSConsumer *NSConsumer::getNSConsumer()\r
         {\r
             ::NSConsumer *nsCon = new ::NSConsumer;\r
@@ -100,6 +120,7 @@ namespace OIC
             ::NSTopicLL *topics = NSProviderGetConsumerTopics(getConsumerId().c_str());\r
 \r
             std::shared_ptr<NSTopicsList> nsTopics = std::make_shared<NSTopicsList>(topics, false);\r
+            removeTopicLL(topics);\r
             NS_LOG(DEBUG, "getConsumerTopicList - OUT");\r
             return nsTopics;\r
         }\r

diff --git a/service/notification/cpp-wrapper/provider/src/NSProviderService.cpp b/service/notification/cpp-wrapper/provider/src/NSProviderService.cpp
index c6332cc..510b2cd 100755 (executable)
--- a/service/notification/cpp-wrapper/provider/src/NSProviderService.cpp
+++ b/service/notification/cpp-wrapper/provider/src/NSProviderService.cpp
@@ -36,6 +36,26 @@ namespace OIC
 {\r
     namespace Service\r
     {\r
+        namespace\r
+        {\r
+            void removeTopicLL(NSTopicLL * topicHead)\r
+            {\r
+                NSTopicLL * iter = topicHead;\r
+                NSTopicLL * following = NULL;\r
+\r
+                while (iter)\r
+                {\r
+                    following = iter->next;\r
+\r
+                    OICFree(iter->topicName);\r
+                    iter->next = NULL;\r
+                    OICFree(iter);\r
+\r
+                    iter = following;\r
+                }\r
+            }\r
+        }\r
+\r
         void NSProviderService::onConsumerSubscribedCallback(::NSConsumer *consumer)\r
         {\r
             NS_LOG(DEBUG, "onConsumerSubscribedCallback - IN");\r
@@ -264,6 +284,7 @@ namespace OIC
 \r
             std::shared_ptr<NSTopicsList> nsTopics = std::make_shared<NSTopicsList>(topics, false);\r
             NS_LOG(DEBUG, "getRegisteredTopicList - OUT");\r
+            removeTopicLL(topics);\r
             return nsTopics;\r
         }\r
 \r

diff --git a/service/notification/cpp-wrapper/unittest/NSConsumerServiceTest2.cpp b/service/notification/cpp-wrapper/unittest/NSConsumerServiceTest2.cpp
index 3bec0be..fb79774 100644 (file)
--- a/service/notification/cpp-wrapper/unittest/NSConsumerServiceTest2.cpp
+++ b/service/notification/cpp-wrapper/unittest/NSConsumerServiceTest2.cpp
@@ -216,7 +216,7 @@ TEST_F(NotificationServiceConsumerTest, ExpectGetProviderSuccessWithInvalidProvi
 TEST_F(NotificationServiceConsumerTest, ExpectGetProviderSuccessWithValidProviderId)
 {
     ::NSProvider *provider = (::NSProvider *)malloc(sizeof(::NSProvider));
-    strcpy(provider->providerId, "test");
+    strcpy(provider->providerId, "098765432109876543210987654321098765");
     std::string provId;
     provId.assign(provider->providerId, NS_UTILS_UUID_STRING_SIZE - 1);
 
@@ -242,7 +242,7 @@ TEST_F(NotificationServiceConsumerTest, ExpectSuccessSendSyncInfo)
     std::string provId;
 
     ::NSProvider *provider = (::NSProvider *)malloc(sizeof(::NSProvider));
-    strcpy(provider->providerId, "test");
+    strcpy(provider->providerId, "098765432109876543210987654321098765");
     provId.assign(provider->providerId, NS_UTILS_UUID_STRING_SIZE - 1);
 
     std::shared_ptr<OIC::Service::NSProvider> providerTemp = std::make_shared<OIC::Service::NSProvider>
@@ -260,6 +260,10 @@ TEST_F(NotificationServiceConsumerTest, ExpectSuccessSendSyncInfo)
         res = resProvider->sendSyncInfo(msgId, OIC::Service::NSSyncInfo::NSSyncType::NS_SYNC_READ);
     }
     EXPECT_EQ(OIC::Service::NSResult::OK, res);
+
+    OIC::Service::NSConsumerService::getInstance()
+        ->getAcceptedProviders()->removeProvider(provider->providerId);
+    free(provider);
 }
 
 TEST_F(NotificationServiceConsumerTest, ExpectSuccessGetTopicsList)
@@ -267,7 +271,7 @@ TEST_F(NotificationServiceConsumerTest, ExpectSuccessGetTopicsList)
     std::string provId;
 
     ::NSProvider *provider = (::NSProvider *)malloc(sizeof(::NSProvider));
-    strcpy(provider->providerId, "test");
+    strcpy(provider->providerId, "098765432109876543210987654321098765");
     provId.assign(provider->providerId, NS_UTILS_UUID_STRING_SIZE - 1);
 
     std::shared_ptr<OIC::Service::NSProvider> providerTemp = std::make_shared<OIC::Service::NSProvider>
@@ -283,5 +287,11 @@ TEST_F(NotificationServiceConsumerTest, ExpectSuccessGetTopicsList)
     auto topicList = resProvider->getTopicList();
 
     ASSERT_NE(nullptr, topicList) << "Get topics list failure";
+
+    OIC::Service::NSConsumerService::getInstance()
+        ->getAcceptedProviders()->removeProvider(provider->providerId);
+    free(provider);
+
+    OIC::Service::NSConsumerService::getInstance()->stop();
 }
 

diff --git a/service/notification/cpp-wrapper/unittest/NSProviderServiceTest2.cpp b/service/notification/cpp-wrapper/unittest/NSProviderServiceTest2.cpp
index 363990a..c045c4d 100644 (file)
--- a/service/notification/cpp-wrapper/unittest/NSProviderServiceTest2.cpp
+++ b/service/notification/cpp-wrapper/unittest/NSProviderServiceTest2.cpp
@@ -66,23 +66,23 @@ class NotificationProviderServiceTest : public TestWithMock
         NotificationProviderServiceTest() = default;
         ~NotificationProviderServiceTest() = default;
 
-        static void ConsumerSubscribedCallback(std::shared_ptr<OIC::Service::NSConsumer> consumer)
+        static void ConsumerSubscribedCallback(std::shared_ptr<OIC::Service::NSConsumer> )
         {
             std::cout << __func__ << std::endl;
         }
 
-        static void MessageSynchronizedCallback(OIC::Service::NSSyncInfo sync)
+        static void MessageSynchronizedCallback(OIC::Service::NSSyncInfo )
         {
             std::cout << __func__ << std::endl;
         }
 
         static void MessageCallbackFromConsumer(
-            const int &id, const std::string &, const std::string &, const std::string &)
+            const int &, const std::string &, const std::string &, const std::string &)
         {
             std::cout << __func__ << std::endl;
         }
 
-        static void SyncCallbackFromConsumer(const int type, const int syncId)
+        static void SyncCallbackFromConsumer(const int , const int )
         {
             std::cout << __func__ << std::endl;
         }
@@ -236,7 +236,7 @@ TEST_F(NotificationProviderServiceTest, ExpectFailAcceptSubscriptionInvalidPolic
 
     OIC::Service::NSProviderService::getInstance()->start(config);
     ::NSConsumer *consumer = (::NSConsumer *)malloc(sizeof(::NSConsumer));
-    strcpy(consumer->consumerId, "test");
+    strcpy(consumer->consumerId, "098765432109876543210987654321098765");
 
     std::shared_ptr<OIC::Service::NSConsumer> consumerTemp = std::make_shared<OIC::Service::NSConsumer>
             (consumer);
@@ -256,7 +256,7 @@ TEST_F(NotificationProviderServiceTest, ExpectFailSetTopicInvalidPolicy)
     std::string str1("TEST1");
     OIC::Service::NSProviderService::getInstance()->registerTopic(str1);
     ::NSConsumer *consumer = (::NSConsumer *)malloc(sizeof(::NSConsumer));
-    strcpy(consumer->consumerId, "test");
+    strcpy(consumer->consumerId, "098765432109876543210987654321098765");
 
     std::shared_ptr<OIC::Service::NSConsumer> consumerTemp = std::make_shared<OIC::Service::NSConsumer>
             (consumer);
@@ -282,7 +282,7 @@ TEST_F(NotificationProviderServiceTest, ExpectSuccessAcceptSubscription)
 
     OIC::Service::NSProviderService::getInstance()->start(config);
     ::NSConsumer *consumer = (::NSConsumer *)malloc(sizeof(::NSConsumer));
-    strcpy(consumer->consumerId, "test");
+    strcpy(consumer->consumerId, "098765432109876543210987654321098765");
 
     std::shared_ptr<OIC::Service::NSConsumer> consumerTemp = std::make_shared<OIC::Service::NSConsumer>
             (consumer);
@@ -303,7 +303,7 @@ TEST_F(NotificationProviderServiceTest, ExpectSuccessSetTopic)
 
     OIC::Service::NSProviderService::getInstance()->registerTopic(str1);
     ::NSConsumer *consumer = (::NSConsumer *)malloc(sizeof(::NSConsumer));
-    strcpy(consumer->consumerId, "test");
+    strcpy(consumer->consumerId, "098765432109876543210987654321098765");
 
     std::shared_ptr<OIC::Service::NSConsumer> consumerTemp = std::make_shared<OIC::Service::NSConsumer>
             (consumer);
@@ -321,7 +321,7 @@ TEST_F(NotificationProviderServiceTest, ExpectSuccessSetTopic)
 TEST_F(NotificationProviderServiceTest, ExpectSuccessUnSetTopic)
 {
     ::NSConsumer *consumer = (::NSConsumer *)malloc(sizeof(::NSConsumer));
-    strcpy(consumer->consumerId, "test");
+    strcpy(consumer->consumerId, "098765432109876543210987654321098765");
 
     std::shared_ptr<OIC::Service::NSConsumer> consumerTemp = std::make_shared<OIC::Service::NSConsumer>
             (consumer);
@@ -376,7 +376,7 @@ TEST_F(NotificationProviderServiceTest, ExpectEqualSetConsumerTopicsAndGetConsum
     std::string str1("TEST1");
     std::string str2("TEST2");
     ::NSConsumer *consumer = (::NSConsumer *)malloc(sizeof(::NSConsumer));
-    strcpy(consumer->consumerId, "test");
+    strcpy(consumer->consumerId, "098765432109876543210987654321098765");
 
     std::shared_ptr<OIC::Service::NSConsumer> consumerTemp = std::make_shared<OIC::Service::NSConsumer>
             (consumer);

diff --git a/service/notification/src/consumer/NSConsumerDiscovery.c b/service/notification/src/consumer/NSConsumerDiscovery.c
index 28c137e..3fea1fb 100644 (file)
--- a/service/notification/src/consumer/NSConsumerDiscovery.c
+++ b/service/notification/src/consumer/NSConsumerDiscovery.c
@@ -68,7 +68,7 @@ OCStackApplicationResult NSConsumerPresenceListener(
         memcpy(addr, clientResponse->addr, sizeof(OCDevAddr));
 
         NSTask * task = NSMakeTask(TASK_CONSUMER_PROVIDER_DELETED, addr);
-        NS_VERIFY_NOT_NULL(task, OC_STACK_KEEP_TRANSACTION);
+        NS_VERIFY_NOT_NULL_WITH_POST_CLEANING(task, OC_STACK_KEEP_TRANSACTION, NSOICFree(addr));
 
         NSConsumerPushEvent(task);
     }
@@ -76,6 +76,7 @@ OCStackApplicationResult NSConsumerPresenceListener(
     else if (payload->trigger == OC_PRESENCE_TRIGGER_CREATE)
     {
         NS_LOG(DEBUG, "started presence or resource is created.");
+        NS_VERIFY_NOT_NULL(clientResponse->addr, OC_STACK_KEEP_TRANSACTION);
         NSInvokeRequest(NULL, OC_REST_DISCOVER, clientResponse->addr,
             NS_DISCOVER_QUERY, NULL, NSProviderDiscoverListener, NULL, NULL,
             clientResponse->addr->adapter);

diff --git a/service/notification/src/provider/NSProviderListener.c b/service/notification/src/provider/NSProviderListener.c
index 5d9adce..b6e7872 100644 (file)
--- a/service/notification/src/provider/NSProviderListener.c
+++ b/service/notification/src/provider/NSProviderListener.c
@@ -110,6 +110,7 @@ OCEntityHandlerResult NSEntityHandlerMessageCb(OCEntityHandlerFlag flag,
                     && strcmp(reqInterface, NS_INTERFACE_READ) != 0)\r
             {\r
                 NS_LOG(ERROR, "Invalid interface");\r
+                NSOICFree(reqInterface);\r
                 return ehResult;\r
             }\r
             ehResult = OC_EH_OK;\r
@@ -185,6 +186,7 @@ OCEntityHandlerResult NSEntityHandlerSyncCb(OCEntityHandlerFlag flag,
                     && strcmp(reqInterface, NS_INTERFACE_READWRITE) != 0)\r
             {\r
                 NS_LOG(ERROR, "Invalid interface");\r
+                NSOICFree(reqInterface);\r
                 return ehResult;\r
             }\r
 \r
@@ -279,6 +281,7 @@ OCEntityHandlerResult NSEntityHandlerTopicCb(OCEntityHandlerFlag flag,
                     && strcmp(reqInterface, NS_INTERFACE_READWRITE) != 0)\r
             {\r
                 NS_LOG(ERROR, "Invalid interface");\r
+                NSOICFree(reqInterface);\r
                 return ehResult;\r
             }\r
             // send consumer's interesting topic list if consumer id exists\r

diff --git a/service/notification/src/provider/NSProviderSubscription.c b/service/notification/src/provider/NSProviderSubscription.c
index 220dced..d5d54e9 100644 (file)
--- a/service/notification/src/provider/NSProviderSubscription.c
+++ b/service/notification/src/provider/NSProviderSubscription.c
@@ -257,6 +257,7 @@ NSResult NSSendResponse(const char * id, bool accepted)
     if (NSPutMessageResource(NULL, &rHandle) != NS_OK)\r
     {\r
         NS_LOG(ERROR, "Fail to put notification resource");\r
+        OCRepPayloadDestroy(payload);\r
         return NS_ERROR;\r
     }\r
 \r
@@ -270,6 +271,7 @@ NSResult NSSendResponse(const char * id, bool accepted)
     if (element == NULL)\r
     {\r
         NS_LOG(ERROR, "element is NULL");\r
+        OCRepPayloadDestroy(payload);\r
         return NS_ERROR;\r
     }\r
 \r

diff --git a/service/notification/src/provider/NSProviderTopic.c b/service/notification/src/provider/NSProviderTopic.c
index 4298939..9e94558 100644 (file)
--- a/service/notification/src/provider/NSProviderTopic.c
+++ b/service/notification/src/provider/NSProviderTopic.c
@@ -133,6 +133,7 @@ NSResult NSSendTopicUpdation()
     if (NSPutMessageResource(NULL, &rHandle) != NS_OK)
     {
         NS_LOG(ERROR, "Fail to put message resource");
+        OCRepPayloadDestroy(payload);
         return NS_ERROR;
     }
 
@@ -164,6 +165,7 @@ NSResult NSSendTopicUpdation()
     if (!obCount)
     {
         NS_LOG(ERROR, "observer count is zero");
+        OCRepPayloadDestroy(payload);
         return NS_ERROR;
     }
 
@@ -196,6 +198,7 @@ NSResult NSSendTopicUpdationToConsumer(char *consumerId)
     if (NSPutMessageResource(NULL, &rHandle) != NS_OK)
     {
         NS_LOG(ERROR, "Fail to put message resource");
+        OCRepPayloadDestroy(payload);
         return NS_ERROR;
     }
 
@@ -208,6 +211,7 @@ NSResult NSSendTopicUpdationToConsumer(char *consumerId)
     if (element == NULL)
     {
         NS_LOG(ERROR, "element is NULL");
+        OCRepPayloadDestroy(payload);
         return NS_ERROR;
     }
 
@@ -526,6 +530,7 @@ void * NSTopicSchedule(void * ptr)
                     pthread_mutex_lock(topicSyncResult->mutex);
                     topicSyncResult->result = NSUnregisterTopic(
                             (const char *) topicSyncResult->topicData);
+                    NSOICFree(topicSyncResult->topicData);
                     pthread_cond_signal(topicSyncResult->condition);
                     pthread_mutex_unlock(topicSyncResult->mutex);
                 }

diff --git a/service/notification/unittest/NSConsumerTest2.cpp b/service/notification/unittest/NSConsumerTest2.cpp
index a172ef2..665d89f 100644 (file)
--- a/service/notification/unittest/NSConsumerTest2.cpp
+++ b/service/notification/unittest/NSConsumerTest2.cpp
@@ -169,15 +169,13 @@ namespace
         OCRepPayload * payload = OCRepPayloadCreate();
         EXPECT_NE((void *)NULL, payload);
 
-        std::string msgUri = "/notifiationTest/message";
-        std::string syncUri = "/notifiationTest/sync";
-        std::string topicUri = "/notifiationTest/topic";
+        std::string msgUri = "/notifiation/message";
+        std::string syncUri = "/notifiation/sync";
 
         bool getResult = OCRepPayloadSetPropBool(payload, NS_ATTRIBUTE_POLICY, false);
         getResult &= OCRepPayloadSetPropString(payload, NS_ATTRIBUTE_PROVIDER_ID, testProviderID.c_str());
         getResult &= OCRepPayloadSetPropString(payload, NS_ATTRIBUTE_MESSAGE, msgUri.c_str());
         getResult &= OCRepPayloadSetPropString(payload, NS_ATTRIBUTE_SYNC, syncUri.c_str());
-        getResult &= OCRepPayloadSetPropString(payload, NS_ATTRIBUTE_TOPIC, topicUri.c_str());
         if (getResult == false)
         {
             OCRepPayloadDestroy(payload);
@@ -503,7 +501,7 @@ TEST(NotificationConsumerTest, ExpectSuccessSendSyncInfo)
     uint64_t id = 100;
     type = NS_SYNC_READ;
 
-    auto ret = NSConsumerSendSyncInfo(g_provider->providerId, id, NS_SYNC_DELETED);
+    auto ret = NSConsumerSendSyncInfo((g_provider->providerId)+1, id, NS_SYNC_DELETED);
 
     EXPECT_EQ(NS_OK, ret);
 }
@@ -615,6 +613,7 @@ TEST(NotificationConsumerTest, ExpectUnsubscribeWithPresenceStart)
     OCPresencePayload * payload = OCPresencePayloadCreate(1, 2, OC_PRESENCE_TRIGGER_CREATE, NULL);
     EXPECT_NE((void *)NULL, payload);
     g_testResponse->payload = (OCPayload *)payload;
+    g_testResponse->addr = NULL;
 
     auto ret = NSConsumerPresenceListener(NULL,NULL, g_testResponse);
 
@@ -632,6 +631,9 @@ TEST(NotificationConsumerTest, ExpectUnsubscribeWithPresenceStop)
 
     auto ret = NSConsumerPresenceListener(NULL,NULL, g_testResponse);
 
+    std::unique_lock< std::mutex > lock{ providerChangedLock };
+    providerChanged.wait_for(lock, g_waitForResponse);
+
     EXPECT_EQ(OC_STACK_KEEP_TRANSACTION, ret);
     OCPresencePayloadDestroy(payload);
 

diff --git a/service/notification/unittest/NSProviderTest2.cpp b/service/notification/unittest/NSProviderTest2.cpp
index 98203ad..fb13b87 100644 (file)
--- a/service/notification/unittest/NSProviderTest2.cpp
+++ b/service/notification/unittest/NSProviderTest2.cpp
@@ -130,8 +130,12 @@ namespace
         bool ret = OCRepPayloadSetPropInt(payload, NS_ATTRIBUTE_MESSAGE_ID, id);
         OCUUIdentity provider;
         OC::OCPlatform::getDeviceId(&provider);
+
+        char providerId[UUID_IDENTITY_SIZE] = {0,};
+        OICStrcpy(providerId, UUID_IDENTITY_SIZE, (const char *)provider.id);
+
         ret &= OCRepPayloadSetPropString(payload,
-                     NS_ATTRIBUTE_PROVIDER_ID, (const char *)provider.id);
+                     NS_ATTRIBUTE_PROVIDER_ID, (const char*)providerId);
         ret &= OCRepPayloadSetPropInt(payload, NS_ATTRIBUTE_STATE, NS_SYNC_READ);
         EXPECT_EQ(true, ret);
 
@@ -307,13 +311,17 @@ TEST(NotificationProviderTest, ExpectCallbackReceiveSync)
     int type = NS_SYNC_READ;
 
     OCEntityHandlerFlag flag = OC_REQUEST_FLAG;
-    NSEntityHandlerSyncCb(flag, getPostSyncEntityRequest(id), NULL);
+    auto request = getPostSyncEntityRequest(id);
+    NSEntityHandlerSyncCb(flag, request, NULL);
 
     std::unique_lock< std::mutex > lock{ responseProviderSyncLock };
     responseProviderSync.wait_for(lock, g_waitForResponse);
 
     EXPECT_EQ(expectedMsgId, id);
     EXPECT_EQ(expectedSyncType, type);
+
+    OCRepPayloadDestroy((OCRepPayload *)request->payload);
+    free(request);
 }
 
 TEST(NotificationProviderTest, ExpectSuccessSetTopics)