Use virtual inheritance in FileSystem hierarchy.

rockset · Dec 22, 2023 · 1d6d20f · 1d6d20f
1 parent 6700da7
commit 1d6d20f
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 13 deletions.
diff --git a/cloud/cloud_file_system.cc b/cloud/cloud_file_system.cc
@@ -547,7 +547,7 @@ Status CloudFileSystem::CreateFromString(
   }
 
   if (s.ok()) {
-    result->reset(static_cast<CloudFileSystem*>(fs.release()));
+    result->reset(dynamic_cast<CloudFileSystem*>(fs.release()));
   }
 
   return s;
@@ -603,7 +603,7 @@ Status CloudFileSystem::CreateFromString(
   }
 
   if (s.ok()) {
-    result->reset(static_cast<CloudFileSystem*>(fs.release()));
+    result->reset(dynamic_cast<CloudFileSystem*>(fs.release()));
   }
 
   return s;
@@ -628,7 +628,7 @@ Status CloudFileSystem::NewAwsFileSystem(
   Status st = AwsFileSystem::NewAwsFileSystem(base_fs, options, logger, cfs);
   if (st.ok()) {
     // store a copy of the logger
-    auto* cloud = static_cast<CloudFileSystemImpl*>(*cfs);
+    auto* cloud = dynamic_cast<CloudFileSystemImpl*>(*cfs);
     cloud->info_log_ = logger;
 
     // start the purge thread only if there is a destination bucket

diff --git a/cloud/db_cloud_test.cc b/cloud/db_cloud_test.cc
@@ -353,11 +353,11 @@ class CloudTest : public testing::Test {
 
   CloudFileSystem* GetCloudFileSystem() const {
     EXPECT_TRUE(aenv_);
-    return static_cast<CloudFileSystem*>(aenv_->GetFileSystem().get());
+    return dynamic_cast<CloudFileSystem*>(aenv_->GetFileSystem().get());
   }
   CloudFileSystemImpl* GetCloudFileSystemImpl() const {
     EXPECT_TRUE(aenv_);
-    return static_cast<CloudFileSystemImpl*>(aenv_->GetFileSystem().get());
+    return dynamic_cast<CloudFileSystemImpl*>(aenv_->GetFileSystem().get());
   }
 
   DBImpl* GetDBImpl() const {
@@ -861,7 +861,7 @@ TEST_F(CloudTest, DISABLED_TrueClone) {
     ASSERT_TRUE(value.compare("World") == 0);
 
     // Assert that there are no redundant sst files
-    auto* cimpl = static_cast<CloudFileSystemImpl*>(env->GetFileSystem().get());
+    auto* cimpl = dynamic_cast<CloudFileSystemImpl*>(env->GetFileSystem().get());
     std::vector<std::string> to_be_deleted;
     ASSERT_OK(
         cimpl->FindObsoleteFiles(cimpl->GetSrcBucketName(), &to_be_deleted));
@@ -1063,7 +1063,7 @@ TEST_F(CloudTest, Savepoint) {
     cloud_db->GetLiveFilesMetaData(&flist);
     ASSERT_TRUE(flist.size() == 1);
 
-    auto* cimpl = static_cast<CloudFileSystemImpl*>(env->GetFileSystem().get());
+    auto* cimpl = dynamic_cast<CloudFileSystemImpl*>(env->GetFileSystem().get());
     auto remapped_fname = cimpl->RemapFilename(flist[0].name);
     // source path
     std::string spath = cimpl->GetSrcObjectPath() + "/" + remapped_fname;

diff --git a/db/db_test_util.cc b/db/db_test_util.cc
@@ -157,7 +157,7 @@ DBTestBase::~DBTestBase() {
 
 #ifndef ROCKSDB_LITE
 #ifdef USE_AWS
-  auto* cfs = static_cast<CloudFileSystem*>(s3_env_->GetFileSystem().get());
+  auto* cfs = dynamic_cast<CloudFileSystem*>(s3_env_->GetFileSystem().get());
   cfs->GetStorageProvider()->EmptyBucket(cfs->GetSrcBucketName(),
                                          cfs->GetSrcObjectPath());
 #endif  // USE_AWS
@@ -826,7 +826,7 @@ void DBTestBase::Destroy(const Options& options, bool delete_cf_paths) {
   ASSERT_OK(DestroyDB(dbname_, options, column_families));
 #ifdef USE_AWS
   if (s3_env_) {
-    auto* cfs = static_cast<CloudFileSystem*>(s3_env_->GetFileSystem().get());
+    auto* cfs = dynamic_cast<CloudFileSystem*>(s3_env_->GetFileSystem().get());
     auto st = cfs->GetStorageProvider()->EmptyBucket(cfs->GetSrcBucketName(),
                                                      dbname_);
     ASSERT_TRUE(st.ok() || st.IsNotFound());

diff --git a/include/rocksdb/cloud/cloud_file_system.h b/include/rocksdb/cloud/cloud_file_system.h
@@ -494,7 +494,7 @@ struct CloudManifestDelta {
 // NOTE: The AWS SDK must be initialized before the CloudFileSystem is
 // constructed, and remain active (Aws::ShutdownAPI() not called) as long as any
 // CloudFileSystem objects exist.
-class CloudFileSystem : public FileSystem {
+class CloudFileSystem : public virtual FileSystem {
  protected:
   CloudFileSystemOptions cloud_fs_options;
   std::shared_ptr<FileSystem> base_fs_;  // The underlying file system

diff --git a/include/rocksdb/cloud/cloud_file_system_impl.h b/include/rocksdb/cloud/cloud_file_system_impl.h
@@ -236,7 +236,6 @@ class CloudFileSystemImpl : public CloudFileSystem {
         "CloudFileSystemImpl::IsDirectory() not supported.");
   }
 
-
   IOStatus Poll(std::vector<void*>& io_handles,
                 size_t /*min_completions*/) override {
     for (auto* handle : io_handles) {

diff --git a/include/rocksdb/customizable.h b/include/rocksdb/customizable.h
@@ -135,7 +135,7 @@ class Customizable : public Configurable {
   template <typename T>
   T* CheckedCast() {
     if (IsInstanceOf(T::kClassName())) {
-      return static_cast<T*>(this);
+      return dynamic_cast<T*>(this);
     } else {
       auto inner = const_cast<Customizable*>(Inner());
       if (inner != nullptr) {

diff --git a/include/rocksdb/file_system.h b/include/rocksdb/file_system.h
@@ -1380,7 +1380,7 @@ class FSDirectory {
 // An implementation of Env that forwards all calls to another Env.
 // May be useful to clients who wish to override just part of the
 // functionality of another Env.
-class FileSystemWrapper : public FileSystem {
+class FileSystemWrapper : public virtual FileSystem {
  public:
   // Initialize an EnvWrapper that delegates all calls to *t
   explicit FileSystemWrapper(const std::shared_ptr<FileSystem>& t);