Use unique_ptr for listBundles API return

listBundles now returns a list with unique_ptr references to RCSBundleInfo objects.
The change clarifies the responsibilty for memory management.

Change-Id: I79f4d70cd6f219f8024b4f2dae548b5ecac606ac
Signed-off-by: Markus Jung <markus.jung@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/3601
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Madan Lanka <lanka.madan@samsung.com>

diff --git a/service/resource-container/examples/ContainerSample.cpp b/service/resource-container/examples/ContainerSample.cpp
index 5706024..5d029b9 100644 (file)
--- a/service/resource-container/examples/ContainerSample.cpp
+++ b/service/resource-container/examples/ContainerSample.cpp
@@ -85,8 +85,8 @@ int main()
     std::map<string, string> bundleParams;
     container->addBundle("oic.bundle.hueSample", "", "libHueBundle.so", "huesample", bundleParams);
 
-    std::list<RCSBundleInfo *> bundles = container->listBundles();
-    std::list<RCSBundleInfo *>::iterator bundleIt;
+    std::list<unique_ptr<RCSBundleInfo>> bundles = container->listBundles();
+    std::list<unique_ptr<RCSBundleInfo>>::iterator bundleIt;
 
     cout << "\t>>> bundle list size : " << bundles.size() << endl;
     for (bundleIt = bundles.begin(); bundleIt != bundles.end(); bundleIt++)

diff --git a/service/resource-container/include/RCSBundleInfo.h b/service/resource-container/include/RCSBundleInfo.h
index 5238df2..31d2902 100644 (file)
--- a/service/resource-container/include/RCSBundleInfo.h
+++ b/service/resource-container/include/RCSBundleInfo.h
@@ -82,12 +82,12 @@ namespace OIC
                 */
                 virtual const std::string &getVersion() = 0;
 
-
+                RCSBundleInfo();
+                virtual ~RCSBundleInfo();
             protected:
                 std::string m_ID, m_path, m_version;
 
-                RCSBundleInfo();
-                virtual ~RCSBundleInfo();
+
         };
     }
 }

diff --git a/service/resource-container/include/RCSResourceContainer.h b/service/resource-container/include/RCSResourceContainer.h
index cd47e7d..f2feee9 100644 (file)
--- a/service/resource-container/include/RCSResourceContainer.h
+++ b/service/resource-container/include/RCSResourceContainer.h
@@ -31,6 +31,7 @@
 #include <vector>
 #include <map>
 #include <list>
+#include <memory>
 
 #include "RCSBundleInfo.h"
 
@@ -64,13 +65,13 @@ namespace OIC
                 // list of bundle ids
                 /**
                 * API for getting the list of all bundles in the container.
-                * The caller receives a copy of the bundle information
-                * and is also responsible for freeing the memory.
+                * The returned list and the contained bundle information are a copy
+                * and will not be updated by the resource container.
                 *
                 * @return List of BundleInfo pointer each associated with a bundle
                 *
                 */
-                virtual std::list<RCSBundleInfo *> listBundles() = 0;
+                virtual std::list<std::unique_ptr<RCSBundleInfo>> listBundles() = 0;
                 /**
                  * API for starting the bundle.
                  *

diff --git a/service/resource-container/src/ResourceContainerImpl.cpp b/service/resource-container/src/ResourceContainerImpl.cpp
index 665cd2a..aaff441 100644 (file)
--- a/service/resource-container/src/ResourceContainerImpl.cpp
+++ b/service/resource-container/src/ResourceContainerImpl.cpp
@@ -518,16 +518,16 @@ namespace OIC
             }
         }
 
-        std::list< RCSBundleInfo * > ResourceContainerImpl::listBundles()
+        std::list<std::unique_ptr<RCSBundleInfo>> ResourceContainerImpl::listBundles()
         {
-            std::list< RCSBundleInfo * > ret;
+            std::list<std::unique_ptr<RCSBundleInfo> > ret;
             for (std::map< std::string, BundleInfoInternal * >::iterator it = m_bundles.begin();
                  it != m_bundles.end(); ++it)
             {
                 {
-                    BundleInfoInternal *bundleInfo = new BundleInfoInternal();
+                    std::unique_ptr<BundleInfoInternal> bundleInfo(new BundleInfoInternal);
                     (bundleInfo)->setBundleInfo(it->second);
-                    ret.push_back((RCSBundleInfo *) bundleInfo);
+                    ret.push_back(std::move(bundleInfo));
                 }
             }
             return ret;

diff --git a/service/resource-container/src/ResourceContainerImpl.h b/service/resource-container/src/ResourceContainerImpl.h
index 5dbcc32..e141fec 100644 (file)
--- a/service/resource-container/src/ResourceContainerImpl.h
+++ b/service/resource-container/src/ResourceContainerImpl.h
@@ -91,7 +91,7 @@ namespace OIC
                                std::map< string, string > params);
                 void removeBundle(const std::string &bundleId);
 
-                std::list< RCSBundleInfo * > listBundles();
+                std::list<std::unique_ptr<RCSBundleInfo>> listBundles();
 
                 void addResourceConfig(const std::string &bundleId, const std::string &resourceUri,
                                        std::map< string, string > params);

diff --git a/service/resource-container/unittests/ResourceContainerTest.cpp b/service/resource-container/unittests/ResourceContainerTest.cpp
index d867888..3b99bbf 100644 (file)
--- a/service/resource-container/unittests/ResourceContainerTest.cpp
+++ b/service/resource-container/unittests/ResourceContainerTest.cpp
@@ -25,6 +25,7 @@
 #include <string>
 #include <map>
 #include <vector>
+#include <memory>
 
 #include <UnitTestHelper.h>
 
@@ -72,6 +73,25 @@ void getCurrentPath(std::string *pPath)
     pPath->append(buffer);
 }
 
+template<typename Derived, typename Base, typename Del>
+std::unique_ptr<Derived, Del>
+static_unique_ptr_cast( std::unique_ptr<Base, Del>&& p )
+{
+    auto d = static_cast<Derived *>(p.release());
+    return std::unique_ptr<Derived, Del>(d, std::move(p.get_deleter()));
+}
+
+template<typename Derived, typename Base, typename Del>
+std::unique_ptr<Derived, Del>
+dynamic_unique_ptr_cast( std::unique_ptr<Base, Del>&& p )
+{
+    if(Derived *result = dynamic_cast<Derived *>(p.get())) {
+        p.release();
+        return std::unique_ptr<Derived, Del>(result, std::move(p.get_deleter()));
+    }
+    return std::unique_ptr<Derived, Del>(nullptr, p.get_deleter());
+}
+
 /*Fake bundle resource class for testing*/
 class TestBundleResource: public BundleResource
 {
@@ -127,9 +147,11 @@ TEST_F(ResourceContainerTest, BundleLoadedWhenContainerStartedWithValidConfigFil
     m_pResourceContainer->startContainer(m_strConfigPath);
 
     EXPECT_GT(m_pResourceContainer->listBundles().size(), (unsigned int) 0);
-    EXPECT_TRUE(((BundleInfoInternal *)(*m_pResourceContainer->listBundles().begin()))->isLoaded());
+    unique_ptr<RCSBundleInfo> first = std::move(*m_pResourceContainer->listBundles().begin());
+    unique_ptr<BundleInfoInternal> firstInternal(static_cast<BundleInfoInternal*>(first.release()));
+    EXPECT_TRUE( firstInternal->isLoaded() );
     EXPECT_NE(nullptr,
-              ((BundleInfoInternal *)( *m_pResourceContainer->listBundles().begin()))->getBundleHandle());
+               firstInternal->getBundleHandle());
 
     m_pResourceContainer->stopContainer();
 }
@@ -139,10 +161,10 @@ TEST_F(ResourceContainerTest, BundleActivatedWhenContainerStartedWithValidConfig
     m_pResourceContainer->startContainer(m_strConfigPath);
 
     EXPECT_GT(m_pResourceContainer->listBundles().size(), (unsigned int) 0);
-    EXPECT_TRUE(
-        ((BundleInfoInternal *)(*m_pResourceContainer->listBundles().begin()))->isActivated());
-    EXPECT_NE(nullptr,
-              ((BundleInfoInternal *)( *m_pResourceContainer->listBundles().begin()))->getBundleActivator());
+    unique_ptr<RCSBundleInfo> first = std::move(*m_pResourceContainer->listBundles().begin());
+    unique_ptr<BundleInfoInternal> firstInternal(static_cast<BundleInfoInternal*>(first.release()));
+    EXPECT_TRUE(firstInternal->isActivated());
+    EXPECT_NE(nullptr,firstInternal->getBundleActivator());
 
     m_pResourceContainer->stopContainer();
 }
@@ -174,8 +196,9 @@ TEST_F(ResourceContainerTest, BundleStoppedWithStartBundleAPI)
     m_pResourceContainer->startContainer(m_strConfigPath);
     m_pResourceContainer->stopBundle("oic.bundle.test");
 
-    EXPECT_FALSE(
-        ((BundleInfoInternal *)(*m_pResourceContainer->listBundles().begin()))->isActivated());
+    unique_ptr<RCSBundleInfo> first = std::move(*m_pResourceContainer->listBundles().begin());
+    unique_ptr<BundleInfoInternal> firstInternal(static_cast<BundleInfoInternal*>(first.release()));
+    EXPECT_FALSE(firstInternal->isActivated());
 
     m_pResourceContainer->stopContainer();
 }
@@ -185,9 +208,9 @@ TEST_F(ResourceContainerTest, BundleStartedWithStartBundleAPI)
     m_pResourceContainer->startContainer(m_strConfigPath);
     m_pResourceContainer->stopBundle("oic.bundle.test");
     m_pResourceContainer->startBundle("oic.bundle.test");
-
-    EXPECT_TRUE(
-        ((BundleInfoInternal *)(*m_pResourceContainer->listBundles().begin()))->isActivated());
+    unique_ptr<RCSBundleInfo> first = std::move(*m_pResourceContainer->listBundles().begin());
+    unique_ptr<BundleInfoInternal> firstInternal(static_cast<BundleInfoInternal*>(first.release()));
+    EXPECT_TRUE(firstInternal->isActivated());
 
     m_pResourceContainer->stopContainer();
 }
@@ -195,19 +218,21 @@ TEST_F(ResourceContainerTest, BundleStartedWithStartBundleAPI)
 TEST_F(ResourceContainerTest, AddNewSoBundleToContainer)
 {
     std::map<string, string> bundleParams;
-    std::list<RCSBundleInfo *> bundles;
+    std::list<std::unique_ptr<RCSBundleInfo>> bundles;
 
     bundles = m_pResourceContainer->listBundles();
     m_pResourceContainer->addBundle("oic.bundle.test", "", "libTestBundle.so", "test", bundleParams);
 
     EXPECT_EQ(bundles.size() + 1, m_pResourceContainer->listBundles().size());
-    EXPECT_TRUE(((BundleInfoInternal *)(*m_pResourceContainer->listBundles().begin()))->isLoaded());
+    unique_ptr<RCSBundleInfo> first = std::move(*m_pResourceContainer->listBundles().begin());
+    unique_ptr<BundleInfoInternal> firstInternal(static_cast<BundleInfoInternal*>(first.release()));
+    EXPECT_TRUE(firstInternal->isLoaded());
 }
 
 TEST_F(ResourceContainerTest, RemoveSoBundleFromContainer)
 {
     std::map<string, string> bundleParams;
-    std::list<RCSBundleInfo *> bundles;
+    std::list<std::unique_ptr<RCSBundleInfo>> bundles;
 
     bundles = m_pResourceContainer->listBundles();
     m_pResourceContainer->removeBundle("oic.bundle.test");
@@ -218,7 +243,7 @@ TEST_F(ResourceContainerTest, RemoveSoBundleFromContainer)
 TEST_F(ResourceContainerTest, AddBundleAlreadyRegistered)
 {
     std::map<string, string> bundleParams;
-    std::list<RCSBundleInfo *> bundles;
+    std::list<std::unique_ptr<RCSBundleInfo> > bundles;
 
     m_pResourceContainer->addBundle("oic.bundle.test", "", "libTestBundle.so", "test", bundleParams);
     bundles = m_pResourceContainer->listBundles();