os: let ObjectStore::create() return unique_ptr<>

instead of returning a raw pointer of ObjectStore, let
`ObjectStore::create()` return a `std::unique_ptr<ObjectStore>`.

less error prune this way.

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/ceph_osd.cc b/src/ceph_osd.cc
index 333e0de9a107cf727b304bab2f22dd2397a24ee3..b6278fa72bc4e90f534ba7340dce060ade457759 100644 (file)
--- a/src/ceph_osd.cc
+++ b/src/ceph_osd.cc
@@ -323,11 +323,11 @@ int main(int argc, const char **argv)
 
   std::string journal_path = g_conf().get_val<std::string>("osd_journal");
   uint32_t flags = g_conf().get_val<uint64_t>("osd_os_flags");
-  ObjectStore *store = ObjectStore::create(g_ceph_context,
-                                          store_type,
-                                          data_path,
-                                          journal_path,
-                                           flags);
+  std::unique_ptr<ObjectStore> store = ObjectStore::create(g_ceph_context,
+                                                          store_type,
+                                                          data_path,
+                                                          journal_path,
+                                                          flags);
   if (!store) {
     derr << "unable to create object store" << dendl;
     forker.exit(-ENODEV);
@@ -369,7 +369,7 @@ int main(int argc, const char **argv)
       forker.exit(-EINVAL);
     }
 
-    int err = OSD::mkfs(g_ceph_context, store, g_conf().get_val<uuid_d>("fsid"),
+    int err = OSD::mkfs(g_ceph_context, store.release(), g_conf().get_val<uuid_d>("fsid"),
                         whoami, osdspec_affinity);
     if (err < 0) {
       derr << TEXT_RED << " ** ERROR: error creating empty object store in "
@@ -438,7 +438,7 @@ int main(int argc, const char **argv)
         << " for object store " << data_path
         << dendl;
 flushjournal_out:
-    delete store;
+    store.reset();
     forker.exit(err < 0 ? 1 : 0);
   }
   if (dump_journal) {
@@ -477,7 +477,7 @@ flushjournal_out:
   uuid_d cluster_fsid, osd_fsid;
   ceph_release_t require_osd_release = ceph_release_t::unknown;
   int w;
-  int r = OSD::peek_meta(store, &magic, &cluster_fsid, &osd_fsid, &w,
+  int r = OSD::peek_meta(store.get(), &magic, &cluster_fsid, &osd_fsid, &w,
                         &require_osd_release);
   if (r < 0) {
     derr << TEXT_RED << " ** ERROR: unable to open OSD superblock on "
@@ -678,7 +678,7 @@ flushjournal_out:
   }
 
   osdptr = new OSD(g_ceph_context,
-                  store,
+                  store.release(),
                   whoami,
                   ms_cluster,
                   ms_public,

diff --git a/src/os/ObjectStore.cc b/src/os/ObjectStore.cc
index dc606bd2cc7a79ebb7cd8b55ff1b6063d7a8bd50..83288068199ce75f899fe6b1a4727efff70e7861 100644 (file)
--- a/src/os/ObjectStore.cc
+++ b/src/os/ObjectStore.cc
@@ -30,44 +30,45 @@
 
 using std::string;
 
-ObjectStore *ObjectStore::create(CephContext *cct,
-                                const string& type,
-                                const string& data,
-                                const string& journal,
-                                osflagbits_t flags)
+std::unique_ptr<ObjectStore> ObjectStore::create(
+  CephContext *cct,
+  const string& type,
+  const string& data,
+  const string& journal,
+  osflagbits_t flags)
 {
 #ifndef WITH_SEASTAR
   if (type == "filestore") {
-    return new FileStore(cct, data, journal, flags);
+    return std::make_unique<FileStore>(cct, data, journal, flags);
   }
   if (type == "memstore") {
-    return new MemStore(cct, data);
+    return std::make_unique<MemStore>(cct, data);
   }
 #endif
 #if defined(WITH_BLUESTORE)
   if (type == "bluestore") {
-    return new BlueStore(cct, data);
+    return std::make_unique<BlueStore>(cct, data);
   }
 #ifndef WITH_SEASTAR
   if (type == "random") {
     if (rand() % 2) {
-      return new FileStore(cct, data, journal, flags);
+      return std::make_unique<FileStore>(cct, data, journal, flags);
     } else {
-      return new BlueStore(cct, data);
+      return std::make_unique<BlueStore>(cct, data);
     }
   }
 #endif
 #else
 #ifndef WITH_SEASTAR
   if (type == "random") {
-    return new FileStore(cct, data, journal, flags);
+    return std::make_unique<FileStore>(cct, data, journal, flags);
   }
 #endif
 #endif
 #ifndef WITH_SEASTAR
   if (type == "kstore" &&
       cct->check_experimental_feature_enabled("kstore")) {
-    return new KStore(cct, data);
+    return std::make_unique<KStore>(cct, data);
   }
 #endif
   return NULL;

diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h
index bba8627f5f2ab2327afebb087d156b70aa6440ab..4fc921d9b61a44365383bd00f9a4f0e2859361f3 100644 (file)
--- a/src/os/ObjectStore.h
+++ b/src/os/ObjectStore.h
@@ -29,8 +29,9 @@
 
 #include <errno.h>
 #include <sys/stat.h>
-#include <vector>
 #include <map>
+#include <memory>
+#include <vector>
 
 #if defined(__APPLE__) || defined(__FreeBSD__) || defined(__sun) || defined(_WIN32)
 #include <sys/statvfs.h>
@@ -77,11 +78,12 @@ public:
    * @param journal path (or other descriptor) for journal (optional)
    * @param flags which filestores should check if applicable
    */
-  static ObjectStore *create(CephContext *cct,
-                            const std::string& type,
-                            const std::string& data,
-                            const std::string& journal,
-                            osflagbits_t flags = 0);
+  static std::unique_ptr<ObjectStore> create(
+    CephContext *cct,
+    const std::string& type,
+    const std::string& data,
+    const std::string& journal,
+    osflagbits_t flags = 0);
 
   /**
    * probe a block device to learn the uuid of the owning OSD

diff --git a/src/test/fio/fio_ceph_objectstore.cc b/src/test/fio/fio_ceph_objectstore.cc
index 33f900b80fca671c700d241361aae387f5c4cba2..86d38919426a3103b7cc8bb7e30d4fcd3683f104 100644 (file)
--- a/src/test/fio/fio_ceph_objectstore.cc
+++ b/src/test/fio/fio_ceph_objectstore.cc
@@ -408,10 +408,10 @@ Engine::Engine(thread_data* td)
   TracepointProvider::initialize<bluestore_tracepoint_traits>(g_ceph_context);
 
   // create the ObjectStore
-  os.reset(ObjectStore::create(g_ceph_context,
-                               g_conf().get_val<std::string>("osd objectstore"),
-                               g_conf().get_val<std::string>("osd data"),
-                               g_conf().get_val<std::string>("osd journal")));
+  os = ObjectStore::create(g_ceph_context,
+                          g_conf().get_val<std::string>("osd objectstore"),
+                          g_conf().get_val<std::string>("osd data"),
+                          g_conf().get_val<std::string>("osd journal"));
   if (!os)
     throw std::runtime_error("bad objectstore type " + g_conf()->osd_objectstore);
 

diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc
index 50db5749bac4e4e9b206320b35261034b6ec6b70..176d1d48b846883462e0449adfbff176773967e1 100644 (file)
--- a/src/test/objectstore/store_test.cc
+++ b/src/test/objectstore/store_test.cc
@@ -16,9 +16,9 @@
 #include <stdio.h>
 #include <string.h>
 #include <iostream>
+#include <memory>
 #include <time.h>
 #include <sys/mount.h>
-#include <boost/scoped_ptr.hpp>
 #include <boost/random/mersenne_twister.hpp>
 #include <boost/random/uniform_int.hpp>
 #include <boost/random/binomial_distribution.hpp>
@@ -6622,7 +6622,7 @@ INSTANTIATE_TEST_SUITE_P(
 #endif
     "kstore"));
 
-void doMany4KWritesTest(boost::scoped_ptr<ObjectStore>& store,
+void doMany4KWritesTest(ObjectStore* store,
                         unsigned max_objects,
                         unsigned max_ops,
                         unsigned max_object_size,
@@ -6634,7 +6634,7 @@ void doMany4KWritesTest(boost::scoped_ptr<ObjectStore>& store,
   coll_t cid(spg_t(pg_t(0,555), shard_id_t::NO_SHARD));
   store_statfs_t res_stat;
 
-  SyntheticWorkloadState test_obj(store.get(),
+  SyntheticWorkloadState test_obj(store,
                                   &gen,
                                   &rng,
                                   cid,
@@ -6675,7 +6675,7 @@ TEST_P(StoreTestSpecificAUSize, Many4KWritesTest) {
   StartDeferred(0x10000);
 
   const unsigned max_object = 4*1024*1024;
-  doMany4KWritesTest(store, 1, 1000, max_object, 4*1024, 0);
+  doMany4KWritesTest(store.get(), 1, 1000, max_object, 4*1024, 0);
 }
 
 TEST_P(StoreTestSpecificAUSize, Many4KWritesNoCSumTest) {
@@ -6686,7 +6686,7 @@ TEST_P(StoreTestSpecificAUSize, Many4KWritesNoCSumTest) {
   g_ceph_context->_conf.apply_changes(nullptr);
   const unsigned max_object = 4*1024*1024;
 
-  doMany4KWritesTest(store, 1, 1000, max_object, 4*1024, 0 );
+  doMany4KWritesTest(store.get(), 1, 1000, max_object, 4*1024, 0 );
 }
 
 TEST_P(StoreTestSpecificAUSize, TooManyBlobsTest) {
@@ -6694,7 +6694,7 @@ TEST_P(StoreTestSpecificAUSize, TooManyBlobsTest) {
     return;
   StartDeferred(0x10000);
   const unsigned max_object = 4*1024*1024;
-  doMany4KWritesTest(store, 1, 1000, max_object, 4*1024, 0);
+  doMany4KWritesTest(store.get(), 1, 1000, max_object, 4*1024, 0);
 }
 
 #if defined(WITH_BLUESTORE)

diff --git a/src/test/objectstore/store_test_fixture.cc b/src/test/objectstore/store_test_fixture.cc
index e343f96156da7d272d995a5a31aead064eb228ac..2b26207d4e51b5741a0bcf3c5b77db739f58dd8f 100644 (file)
--- a/src/test/objectstore/store_test_fixture.cc
+++ b/src/test/objectstore/store_test_fixture.cc
@@ -40,10 +40,10 @@ void StoreTestFixture::SetUp()
   }
   ASSERT_EQ(0, r);
 
-  store.reset(ObjectStore::create(g_ceph_context,
-                                  type,
-                                  data_dir,
-                                  string("store_test_temp_journal")));
+  store = ObjectStore::create(g_ceph_context,
+                              type,
+                              data_dir,
+                              "store_test_temp_journal");
   if (!store) {
     cerr << __func__ << ": objectstore type " << type << " doesn't exist yet!" << std::endl;
   }
@@ -113,10 +113,10 @@ void StoreTestFixture::CloseAndReopen() {
   EXPECT_EQ(0, r);
   ch.reset(nullptr);
   store.reset(nullptr);
-  store.reset(ObjectStore::create(g_ceph_context,
-                                  type,
-                                  data_dir,
-                                  string("store_test_temp_journal")));
+  store = ObjectStore::create(g_ceph_context,
+                              type,
+                              data_dir,
+                              "store_test_temp_journal");
   if (!store) {
     cerr << __func__ << ": objectstore type " << type << " failed to reopen!" << std::endl;
   }

diff --git a/src/test/objectstore/store_test_fixture.h b/src/test/objectstore/store_test_fixture.h
index 3b7971d7cdee57f210eb40fd7c77599d31706bd5..3f25fd493d0d3c8e7876f91a070943b151b4b37f 100644 (file)
--- a/src/test/objectstore/store_test_fixture.h
+++ b/src/test/objectstore/store_test_fixture.h
@@ -1,6 +1,6 @@
 #include <string>
 #include <stack>
-#include <boost/scoped_ptr.hpp>
+#include <memory>
 #include <gtest/gtest.h>
 #include "common/config_fwd.h"
 
@@ -16,7 +16,7 @@ class StoreTestFixture : virtual public ::testing::Test {
   std::string orig_death_test_style;
 
 public:
-  boost::scoped_ptr<ObjectStore> store;
+  std::unique_ptr<ObjectStore> store;
   ObjectStore::CollectionHandle ch;
 
   explicit StoreTestFixture(const std::string& type)

diff --git a/src/test/objectstore_bench.cc b/src/test/objectstore_bench.cc
index b04e2d8c141ea66424ff3336ad9d598002683806..bc25c5a8f7f4c639beab4eb83f31cefe221e3252 100644 (file)
--- a/src/test/objectstore_bench.cc
+++ b/src/test/objectstore_bench.cc
@@ -207,11 +207,11 @@ int main(int argc, const char *argv[])
   dout(0) << "repeats " << cfg.repeats << dendl;
   dout(0) << "threads " << cfg.threads << dendl;
 
-  auto os = std::unique_ptr<ObjectStore>(
+  auto os =
       ObjectStore::create(g_ceph_context,
                           g_conf()->osd_objectstore,
                           g_conf()->osd_data,
-                          g_conf()->osd_journal));
+                          g_conf()->osd_journal);
 
   //Checking data folder: create if needed or error if it's not empty
   DIR *dir = ::opendir(g_conf()->osd_data.c_str());

diff --git a/src/test/osd/TestOSDScrub.cc b/src/test/osd/TestOSDScrub.cc
index 45d79a18379429e16f0cd3758f8b21be51643792..77caa4104a8b03d2278728e9ab903b9191f4675b 100644 (file)
--- a/src/test/osd/TestOSDScrub.cc
+++ b/src/test/osd/TestOSDScrub.cc
@@ -55,7 +55,7 @@ public:
 
 TEST(TestOSDScrub, scrub_time_permit) {
   ceph::async::io_context_pool icp(1);
-  ObjectStore *store = ObjectStore::create(g_ceph_context,
+  std::unique_ptr<ObjectStore> store = ObjectStore::create(g_ceph_context,
              g_conf()->osd_objectstore,
              g_conf()->osd_data,
              g_conf()->osd_journal);
@@ -68,7 +68,7 @@ TEST(TestOSDScrub, scrub_time_permit) {
   ms->bind(g_conf()->public_addr);
   MonClient mc(g_ceph_context, icp);
   mc.build_initial_monmap();
-  TestOSDScrub* osd = new TestOSDScrub(g_ceph_context, store, 0, ms, ms, ms, ms, ms, ms, ms, &mc, "", "", icp);
+  TestOSDScrub* osd = new TestOSDScrub(g_ceph_context, store.release(), 0, ms, ms, ms, ms, ms, ms, ms, &mc, "", "", icp);
 
   // These are now invalid
   int err = g_ceph_context->_conf.set_val("osd_scrub_begin_hour", "24");

diff --git a/src/tools/ceph_objectstore_tool.cc b/src/tools/ceph_objectstore_tool.cc
index 9a77a8df29dae36c7a76bd30979645e4190ce9a3..1873b69a31add667cc3c38e090ccca758162f4ba 100644 (file)
--- a/src/tools/ceph_objectstore_tool.cc
+++ b/src/tools/ceph_objectstore_tool.cc
@@ -3513,7 +3513,7 @@ int main(int argc, char **argv)
     }
   }
 
-  ObjectStore *fs = ObjectStore::create(g_ceph_context, type, dpath, jpath, flags);
+  ObjectStore *fs = ObjectStore::create(g_ceph_context, type, dpath, jpath, flags).release();
   if (fs == NULL) {
     cerr << "Unable to create store of type " << type << std::endl;
     return 1;
@@ -3577,14 +3577,14 @@ int main(int argc, char **argv)
       target_type = string(bl.c_str(), bl.length() - 1);  // drop \n
     }
     ::close(fd);
-    ObjectStore *targetfs = ObjectStore::create(
+    unique_ptr<ObjectStore> targetfs = ObjectStore::create(
       g_ceph_context, target_type,
       target_data_path, "", 0);
     if (targetfs == NULL) {
       cerr << "Unable to open store of type " << target_type << std::endl;
       return 1;
     }
-    int r = dup(dpath, fs, target_data_path, targetfs);
+    int r = dup(dpath, fs, target_data_path, targetfs.get());
     if (r < 0) {
       cerr << "dup failed: " << cpp_strerror(r) << std::endl;
       return 1;