fix: replace new/delete with smart pointers

include/pika_binlog.h

@@ -102,9 +102,9 @@ class Binlog {
 
  std::atomic<bool> opened_;
 
- Version* version_ = nullptr;
- pstd::WritableFile* queue_ = nullptr;
- pstd::RWFile* versionfile_ = nullptr;
+ std::unique_ptr<Version> version_;
+ std::unique_ptr<pstd::WritableFile> queue_;
+ std::unique_ptr<pstd::RWFile> versionfile_;
 
  pstd::Mutex mutex_;
 

include/pika_binlog_reader.h

@@ -41,7 +41,7 @@ class PikaBinlogReader {
  uint64_t last_record_offset_ = 0;
 
  std::shared_ptr<Binlog> logger_;
- pstd::SequentialFile* queue_ = nullptr;
+ std::unique_ptr<pstd::SequentialFile> queue_;
 
  char* const backing_store_;
  Slice buffer_;

include/pika_consensus.h

@@ -38,7 +38,7 @@ class Context {
 
  private:
  std::string path_;
- pstd::RWFile* save_ = nullptr;
+ std::unique_ptr<pstd::RWFile> save_;
  // No copying allowed;
  Context(const Context&);
  void operator=(const Context&);

include/pika_partition.h

@@ -62,7 +62,7 @@ class Partition : public std::enable_shared_from_this<Partition> {
  void DbRWLockReader();
  void DbRWUnLock();
 
- pstd::lock::LockMgr* LockMgr();
+ std::shared_ptr<pstd::lock::LockMgr> LockMgr();
 
  void PrepareRsync();
  bool TryUpdateMasterOffset();
@@ -97,7 +97,7 @@ class Partition : public std::enable_shared_from_this<Partition> {
  bool opened_ = false;
 
  std::shared_mutex db_rwlock_;
- pstd::lock::LockMgr* lock_mgr_ = nullptr;
+ std::shared_ptr<pstd::lock::LockMgr> lock_mgr_;
  std::shared_ptr<storage::Storage> db_;
 
  bool full_sync_ = false;
@@ -116,7 +116,7 @@ class Partition : public std::enable_shared_from_this<Partition> {
  void FinishBgsave();
  BgSaveInfo bgsave_info_;
  pstd::Mutex bgsave_protector_;
- storage::BackupEngine* bgsave_engine_;
+ std::unique_ptr<storage::BackupEngine> bgsave_engine_;
 
  // key scan info use
  void InitKeyScan();
@@ -129,3 +129,4 @@ class Partition : public std::enable_shared_from_this<Partition> {
 };
 
 #endif
+

src/pika_binlog.cc

@@ -81,24 +81,24 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size)
  LOG(INFO) << "Binlog: Manifest file not exist, we create a new one.";
 
  profile = NewFileName(filename_, pro_num_);
- s = pstd::NewWritableFile(profile, &queue_);
+ s = pstd::NewWritableFile(profile, queue_);
  if (!s.ok()) {
  LOG(FATAL) << "Binlog: new " << filename_ << " " << s.ToString();
  }
 
- s = pstd::NewRWFile(manifest, &versionfile_);
+ s = pstd::NewRWFile(manifest, versionfile_);
  if (!s.ok()) {
  LOG(FATAL) << "Binlog: new versionfile error " << s.ToString();
  }
 
- version_ = new Version(versionfile_);
+ version_ = std::unique_ptr<Version>(new Version(versionfile_.get()));
  version_->StableSave();
  } else {
  LOG(INFO) << "Binlog: Find the exist file.";
 
- s = pstd::NewRWFile(manifest, &versionfile_);
+ s = pstd::NewRWFile(manifest, versionfile_);
  if (s.ok()) {
- version_ = new Version(versionfile_);
+ version_ = std::unique_ptr<Version>(new Version(versionfile_.get()));
  version_->Init();
  pro_num_ = version_->pro_num_;
 
@@ -110,7 +110,7 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size)
 
  profile = NewFileName(filename_, pro_num_);
  DLOG(INFO) << "Binlog: open profile " << profile;
- s = pstd::AppendWritableFile(profile, &queue_, version_->pro_offset_);
+ s = pstd::AppendWritableFile(profile, queue_, version_->pro_offset_);
  if (!s.ok()) {
  LOG(FATAL) << "Binlog: Open file " << profile << " error " << s.ToString();
  }
@@ -125,10 +125,6 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size)
 Binlog::~Binlog() {
  std::lock_guard l(mutex_);
  Close();
- delete version_;
- delete versionfile_;
-
- delete queue_;
 }
 
 void Binlog::Close() {
@@ -185,15 +181,15 @@ Status Binlog::Put(const char* item, int len) {
  /* Check to roll log file */
  uint64_t filesize = queue_->Filesize();
  if (filesize > file_size_) {
- pstd::WritableFile* queue = nullptr;
+ std::unique_ptr<pstd::WritableFile> queue;
  std::string profile = NewFileName(filename_, pro_num_ + 1);
- s = pstd::NewWritableFile(profile, &queue);
+ s = pstd::NewWritableFile(profile, queue);
  if (!s.ok()) {
  LOG(ERROR) << "Binlog: new " << filename_ << " " << s.ToString();
  return s;
  }
- delete queue_;
- queue_ = queue;
+ queue_.reset();
+ queue_ = std::move(queue);
  pro_num_++;
 
  {
@@ -352,7 +348,7 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset, uint32_t
  pro_offset = 0;
  }
 
- delete queue_;
+ queue_.reset();
 
  std::string init_profile = NewFileName(filename_, 0);
  if (pstd::FileExists(init_profile)) {
@@ -364,8 +360,8 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset, uint32_t
  pstd::DeleteFile(profile);
  }
 
- pstd::NewWritableFile(profile, &queue_);
- Binlog::AppendPadding(queue_, &pro_offset);
+ pstd::NewWritableFile(profile, queue_);
+ Binlog::AppendPadding(queue_.get(), &pro_offset);
 
  pro_num_ = pro_num;
 
@@ -383,7 +379,7 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset, uint32_t
 }
 
 Status Binlog::Truncate(uint32_t pro_num, uint64_t pro_offset, uint64_t index) {
- delete queue_;
+ queue_.reset();
  std::string profile = NewFileName(filename_, pro_num);
  const int fd = open(profile.c_str(), O_RDWR | O_CLOEXEC, 0644);
  if (fd < 0) {
@@ -403,7 +399,7 @@ Status Binlog::Truncate(uint32_t pro_num, uint64_t pro_offset, uint64_t index) {
  version_->StableSave();
  }
 
- Status s = pstd::AppendWritableFile(profile, &queue_, version_->pro_offset_);
+ Status s = pstd::AppendWritableFile(profile, queue_, version_->pro_offset_);
  if (!s.ok()) {
  return s;
  }

src/pika_binlog_reader.cc

@@ -29,7 +29,6 @@ PikaBinlogReader::PikaBinlogReader()
 
 PikaBinlogReader::~PikaBinlogReader() {
  delete[] backing_store_;
- delete queue_;
 }
 
 void PikaBinlogReader::GetReaderStatus(uint32_t* cur_filenum, uint64_t* cur_offset) {
@@ -52,15 +51,15 @@ int PikaBinlogReader::Seek(std::shared_ptr<Binlog> logger, uint32_t filenum, uin
  LOG(WARNING) << confile << " not exits";
  return -1;
  }
- pstd::SequentialFile* readfile;
- if (!pstd::NewSequentialFile(confile, &readfile).ok()) {
+ std::unique_ptr<pstd::SequentialFile> readfile;
+ if (!pstd::NewSequentialFile(confile, readfile).ok()) {
  LOG(WARNING) << "New swquential " << confile << " failed";
  return -1;
  }
  if (queue_) {
- delete queue_;
+ queue_.reset();
  }
- queue_ = readfile;
+ queue_ = std::move(readfile);
  logger_ = logger;
 
  std::lock_guard l(rwlock_);
@@ -251,10 +250,10 @@ Status PikaBinlogReader::Get(std::string* scratch, uint32_t* filenum, uint64_t*
  // Roll to next file need retry;
  if (pstd::FileExists(confile)) {
  DLOG(INFO) << "BinlogSender roll to new binlog" << confile;
- delete queue_;
+ queue_.reset();
  queue_ = nullptr;
 
- pstd::NewSequentialFile(confile, &(queue_));
+ pstd::NewSequentialFile(confile, queue_);
  {
  std::lock_guard l(rwlock_);
  cur_filenum_++;

src/pika_consensus.cc

@@ -20,7 +20,7 @@ extern PikaCmdTableManager* g_pika_cmd_table_manager;
 
 Context::Context(const std::string path) : applied_index_(), path_(path), save_(nullptr) {}
 
-Context::~Context() { delete save_; }
+Context::~Context() {}
 
 Status Context::StableSave() {
  char* p = save_->GetData();
@@ -36,13 +36,13 @@ Status Context::StableSave() {
 
 Status Context::Init() {
  if (!pstd::FileExists(path_)) {
- Status s = pstd::NewRWFile(path_, &save_);
+ Status s = pstd::NewRWFile(path_, save_);
  if (!s.ok()) {
  LOG(FATAL) << "Context new file failed " << s.ToString();
  }
  StableSave();
  } else {
- Status s = pstd::NewRWFile(path_, &save_);
+ Status s = pstd::NewRWFile(path_, save_);
  if (!s.ok()) {
  LOG(FATAL) << "Context new file failed " << s.ToString();
  }

src/pika_meta.cc

@@ -29,11 +29,10 @@ Status PikaMeta::StableSave(const std::vector<TableStruct>& table_structs) {
  std::string tmp_file = local_meta_file;
  tmp_file.append("_tmp");
 
- pstd::RWFile* saver = nullptr;
+ std::unique_ptr<pstd::RWFile> saver;
  pstd::CreatePath(local_meta_path_);
- Status s = pstd::NewRWFile(tmp_file, &saver);
+ Status s = pstd::NewRWFile(tmp_file, saver);
  if (!s.ok()) {
- delete saver;
  LOG(WARNING) << "Open local meta file failed";
  return Status::Corruption("open local meta file failed");
  }
@@ -50,7 +49,6 @@ Status PikaMeta::StableSave(const std::vector<TableStruct>& table_structs) {
 
  std::string meta_str;
  if (!meta.SerializeToString(&meta_str)) {
- delete saver;
  LOG(WARNING) << "Serialize meta string failed";
  return Status::Corruption("serialize meta string failed");
  }
@@ -62,7 +60,6 @@ Status PikaMeta::StableSave(const std::vector<TableStruct>& table_structs) {
  memcpy(p, &meta_str_size, sizeof(uint32_t));
  p += sizeof(uint32_t);
  memcpy(p, meta_str.data(), meta_str.size());
- delete saver;
 
  pstd::DeleteFile(local_meta_file);
  if (pstd::RenameFile(tmp_file, local_meta_file)) {
@@ -80,16 +77,14 @@ Status PikaMeta::ParseMeta(std::vector<TableStruct>* const table_structs) {
  return Status::Corruption("meta file not found");
  }
 
- pstd::RWFile* reader = nullptr;
- Status s = pstd::NewRWFile(local_meta_file, &reader);
+ std::unique_ptr<pstd::RWFile> reader;
+ Status s = pstd::NewRWFile(local_meta_file, reader);
  if (!s.ok()) {
- delete reader;
  LOG(WARNING) << "Open local meta file failed";
  return Status::Corruption("open local meta file failed");
  }
 
  if (reader->GetData() == nullptr) {
- delete reader;
  LOG(WARNING) << "Meta file init error";
  return Status::Corruption("meta file init error");
  }
@@ -98,18 +93,15 @@ Status PikaMeta::ParseMeta(std::vector<TableStruct>* const table_structs) {
  uint32_t meta_size = 0;
  memcpy((char*)(&version), reader->GetData(), sizeof(uint32_t));
  memcpy((char*)(&meta_size), reader->GetData() + sizeof(uint32_t), sizeof(uint32_t));
- char* const buf = new char[meta_size];
+ auto const buf_ptr = std::make_unique<char[]>(meta_size);
+ char* const buf = buf_ptr.get();
  memcpy(buf, reader->GetData() + 2 * sizeof(uint32_t), meta_size);
 
  InnerMessage::PikaMeta meta;
  if (!meta.ParseFromArray(buf, meta_size)) {
- delete[] buf;
- delete reader;
  LOG(WARNING) << "Parse meta string failed";
  return Status::Corruption("parse meta string failed");
  }
- delete[] buf;
- delete reader;
 
  table_structs->clear();
  for (int idx = 0; idx < meta.table_infos_size(); ++idx) {

src/pika_partition.cc

@@ -53,7 +53,7 @@ Partition::Partition(const std::string& table_name, uint32_t partition_id, const
  db_ = std::shared_ptr<storage::Storage>(new storage::Storage());
  rocksdb::Status s = db_->Open(g_pika_server->storage_options(), db_path_);
 
- lock_mgr_ = new pstd::lock::LockMgr(1000, 0, std::make_shared<pstd::lock::MutexFactoryImpl>());
+ lock_mgr_ = std::make_shared<pstd::lock::LockMgr>(1000, 0, std::make_shared<pstd::lock::MutexFactoryImpl>());
 
  opened_ = s.ok() ? true : false;
  assert(db_);
@@ -63,8 +63,6 @@ Partition::Partition(const std::string& table_name, uint32_t partition_id, const
 
 Partition::~Partition() {
  Close();
- delete bgsave_engine_;
- delete lock_mgr_;
 }
 
 void Partition::Leave() {
@@ -121,7 +119,7 @@ void Partition::DbRWLockReader() { db_rwlock_.lock_shared(); }
 
 void Partition::DbRWUnLock() { db_rwlock_.unlock(); }
 
-pstd::lock::LockMgr* Partition::LockMgr() { return lock_mgr_; }
+std::shared_ptr<pstd::lock::LockMgr> Partition::LockMgr() { return lock_mgr_; }
 
 void Partition::PrepareRsync() {
  pstd::DeleteDirIfExist(dbsync_path_);
@@ -366,8 +364,8 @@ bool Partition::InitBgsaveEnv() {
 
 // Prepare bgsave env, need bgsave_protector protect
 bool Partition::InitBgsaveEngine() {
- delete bgsave_engine_;
- rocksdb::Status s = storage::BackupEngine::Open(db().get(), &bgsave_engine_);
+ bgsave_engine_ = nullptr;
+ rocksdb::Status s = storage::BackupEngine::Open(db().get(), bgsave_engine_);
  if (!s.ok()) {
  LOG(WARNING) << partition_name_ << " open backup engine failed " << s.ToString();
  return false;
@@ -498,3 +496,4 @@ Status Partition::GetKeyNum(std::vector<storage::KeyInfo>* key_info) {
  key_scan_info_.duration = time(nullptr) - key_scan_info_.start_time;
  return Status::OK();
 }
+

src/pstd/include/base_conf.h

@@ -9,9 +9,9 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include <memory>
 #include <string>
 #include <vector>
-
 #include "pstd/include/pstd_define.h"
 
 namespace pstd {
@@ -73,7 +73,7 @@ class BaseConf {
  void PushConfItem(const Rep::ConfItem& item);
 
  private:
- Rep* rep_ = nullptr;
+ std::unique_ptr<Rep> rep_;
 
  /*
  * No copy && no assign operator