From 38b974813a5978dc4ec9a5798d0a7c143b1dc46b Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Tue, 6 Oct 2015 10:31:02 +0900 Subject: [PATCH] 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 Reviewed-on: https://gerrit.iotivity.org/gerrit/3551 Tested-by: jenkins-iotivity Reviewed-by: Madan Lanka --- .../examples/ContainerSample.cpp | 4 +- service/resource-container/include/RCSBundleInfo.h | 6 +-- .../include/RCSResourceContainer.h | 7 +-- .../src/ResourceContainerImpl.cpp | 8 ++-- .../resource-container/src/ResourceContainerImpl.h | 2 +- .../unittests/ResourceContainerTest.cpp | 55 ++++++++++++++++------ 6 files changed, 54 insertions(+), 28 deletions(-) diff --git a/service/resource-container/examples/ContainerSample.cpp b/service/resource-container/examples/ContainerSample.cpp index 5706024..5d029b9 100644 --- a/service/resource-container/examples/ContainerSample.cpp +++ b/service/resource-container/examples/ContainerSample.cpp @@ -85,8 +85,8 @@ int main() std::map bundleParams; container->addBundle("oic.bundle.hueSample", "", "libHueBundle.so", "huesample", bundleParams); - std::list bundles = container->listBundles(); - std::list::iterator bundleIt; + std::list> bundles = container->listBundles(); + std::list>::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 --- 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 --- a/service/resource-container/include/RCSResourceContainer.h +++ b/service/resource-container/include/RCSResourceContainer.h @@ -31,6 +31,7 @@ #include #include #include +#include #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 listBundles() = 0; + virtual std::list> 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 --- 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> ResourceContainerImpl::listBundles() { - std::list< RCSBundleInfo * > ret; + std::list > ret; for (std::map< std::string, BundleInfoInternal * >::iterator it = m_bundles.begin(); it != m_bundles.end(); ++it) { { - BundleInfoInternal *bundleInfo = new BundleInfoInternal(); + std::unique_ptr 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 --- 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> 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 --- a/service/resource-container/unittests/ResourceContainerTest.cpp +++ b/service/resource-container/unittests/ResourceContainerTest.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include @@ -72,6 +73,25 @@ void getCurrentPath(std::string *pPath) pPath->append(buffer); } +template +std::unique_ptr +static_unique_ptr_cast( std::unique_ptr&& p ) +{ + auto d = static_cast(p.release()); + return std::unique_ptr(d, std::move(p.get_deleter())); +} + +template +std::unique_ptr +dynamic_unique_ptr_cast( std::unique_ptr&& p ) +{ + if(Derived *result = dynamic_cast(p.get())) { + p.release(); + return std::unique_ptr(result, std::move(p.get_deleter())); + } + return std::unique_ptr(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 first = std::move(*m_pResourceContainer->listBundles().begin()); + unique_ptr firstInternal(static_cast(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 first = std::move(*m_pResourceContainer->listBundles().begin()); + unique_ptr firstInternal(static_cast(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 first = std::move(*m_pResourceContainer->listBundles().begin()); + unique_ptr firstInternal(static_cast(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 first = std::move(*m_pResourceContainer->listBundles().begin()); + unique_ptr firstInternal(static_cast(first.release())); + EXPECT_TRUE(firstInternal->isActivated()); m_pResourceContainer->stopContainer(); } @@ -195,19 +218,21 @@ TEST_F(ResourceContainerTest, BundleStartedWithStartBundleAPI) TEST_F(ResourceContainerTest, AddNewSoBundleToContainer) { std::map bundleParams; - std::list bundles; + std::list> 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 first = std::move(*m_pResourceContainer->listBundles().begin()); + unique_ptr firstInternal(static_cast(first.release())); + EXPECT_TRUE(firstInternal->isLoaded()); } TEST_F(ResourceContainerTest, RemoveSoBundleFromContainer) { std::map bundleParams; - std::list bundles; + std::list> 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 bundleParams; - std::list bundles; + std::list > bundles; m_pResourceContainer->addBundle("oic.bundle.test", "", "libTestBundle.so", "test", bundleParams); bundles = m_pResourceContainer->listBundles(); -- 2.7.4