Minor refactor

- Remove unnecessary parameter and local variable which is no longer used.
- Extract duplicate codes info function.
- Change variable name for readability.
- Adjust code order for readability.

Change-Id: I98050d35f0fd9a256895e698918fc34f8840fad1
Signed-off-by: Junghyun Yeon <jungh.yeon@samsung.com>

diff --git a/src/server/database/db_handle_provider.cc b/src/server/database/db_handle_provider.cc
index 6b3579f..39b5150 100644 (file)
--- a/src/server/database/db_handle_provider.cc
+++ b/src/server/database/db_handle_provider.cc
@@ -57,10 +57,10 @@ uid_t ConvertUID(uid_t uid) {
     return uid;
 }
 
-static const std::string global_parser_memory_db_path =
+static const std::string global_parser_memdb_path =
     "file:parserdb?mode=memory&cache=shared";
 
-static const std::string cert_memory_db_path =
+static const std::string cert_memdb_path =
     "file:certdb?mode=memory&cache=shared";
 
 }  // namespace
@@ -69,42 +69,44 @@ namespace pkgmgr_server {
 namespace database {
 
 std::unordered_map<uid_t, std::unique_ptr<DBHandleProvider>>
-    DBHandleProvider::provider_;
-bool DBHandleProvider::is_memory_global_ = false;
+    DBHandleProvider::provider_map_;
+bool DBHandleProvider::is_global_memdb_set_ = false;
 std::unique_ptr<sqlite3, decltype(sqlite3_close_v2)*>
-    DBHandleProvider::global_parser_memory_db_handle_(nullptr,
-                                                      sqlite3_close_v2);
+    DBHandleProvider::global_parser_memdb_handle_(nullptr,
+                                                  sqlite3_close_v2);
 std::unique_ptr<sqlite3, decltype(sqlite3_close_v2)*>
-    DBHandleProvider::cert_memory_db_handle_(nullptr, sqlite3_close_v2);
-std::string DBHandleProvider::global_parser_file_db_path_;
-std::string DBHandleProvider::cert_file_db_path_;
-std::unordered_set<pid_t> DBHandleProvider::pid_list_;
+    DBHandleProvider::cert_memdb_handle_(nullptr, sqlite3_close_v2);
+std::string DBHandleProvider::global_parser_filedb_path_;
+std::string DBHandleProvider::cert_filedb_path_;
+std::unordered_set<pid_t> DBHandleProvider::writer_pid_list_;
 std::recursive_mutex DBHandleProvider::lock_;
 std::mutex DBHandleProvider::pid_list_lock_;
 
 DBHandleProvider::DBHandleProvider(uid_t uid)
     : uid_(uid),
-      is_memory_(false),
-      parser_memory_db_handle_(nullptr, sqlite3_close_v2) {
+      is_user_memdb_set_(false),
+      parser_memdb_handle_(nullptr, sqlite3_close_v2) {
   char* tmp_path;
 
-  if (global_parser_file_db_path_.empty()) {
+  if (global_parser_filedb_path_.empty()) {
     tmp_path = getUserPkgParserDBPathUID(GetGlobalUID());
-    global_parser_file_db_path_ = tmp_path;
+    global_parser_filedb_path_ = tmp_path;
     free(tmp_path);
+  }
 
+  if (cert_filedb_path_.empty()) {
     tmp_path = getUserPkgCertDBPath();
-    cert_file_db_path_ = tmp_path;
+    cert_filedb_path_ = tmp_path;
     free(tmp_path);
   }
 
   tmp_path = getUserPkgParserDBPathUID(uid_);
-  parser_file_db_path_ = tmp_path;
+  user_parser_filedb_path_ = tmp_path;
   free(tmp_path);
 
-  parser_memory_db_path_ = "file:parserdb" +
-                           std::to_string(static_cast<int>(uid_)) +
-                           "?mode=memory&cache=shared";
+  user_parser_memdb_path_ = "file:parserdb" +
+                            std::to_string(static_cast<int>(uid_)) +
+                            "?mode=memory&cache=shared";
 }
 
 DBHandleProvider& DBHandleProvider::GetInst(uid_t uid) {
@@ -112,7 +114,7 @@ DBHandleProvider& DBHandleProvider::GetInst(uid_t uid) {
   std::unique_lock<std::mutex> u(singleton_lock_);
 
   uid = ConvertUID(uid);
-  auto& prov = provider_[uid];
+  auto& prov = provider_map_[uid];
   if (prov == nullptr)
     prov.reset(new DBHandleProvider(uid));
 
@@ -122,13 +124,13 @@ DBHandleProvider& DBHandleProvider::GetInst(uid_t uid) {
 bool DBHandleProvider::IsCrashedWriteRequest() {
   std::unique_lock<std::mutex> u(pid_list_lock_);
 
-  if (pid_list_.empty())
+  if (writer_pid_list_.empty())
     return false;
 
   bool ret = true;
-  LOG(DEBUG) << "Check process count : " << pid_list_.size();
+  LOG(DEBUG) << "Check process count : " << writer_pid_list_.size();
   std::vector<pid_t> remove_pids;
-  for (pid_t pid : pid_list_) {
+  for (pid_t pid : writer_pid_list_) {
     std::string status_path = "/proc/" + std::to_string(pid) + "/status";
 
     int fd = open(status_path.c_str(), O_RDONLY);
@@ -142,7 +144,7 @@ bool DBHandleProvider::IsCrashedWriteRequest() {
   }
 
   for (pid_t pid : remove_pids)
-    pid_list_.erase(pid);
+    writer_pid_list_.erase(pid);
 
   return ret;
 }
@@ -152,19 +154,21 @@ std::vector<std::pair<std::string, uid_t>> DBHandleProvider::GetParserDBPath(
   std::unique_lock<std::recursive_mutex> u(lock_);
   std::vector<std::pair<std::string, uid_t>> db_path_list;
 
-  if (is_memory_ != is_memory_global_)
-    is_memory_global_ ? SetMemoryMode(pid) : UnsetMemoryMode(pid);
+  if (is_user_memdb_set_ != is_global_memdb_set_)
+    is_global_memdb_set_ ? SetMemoryMode(pid) : UnsetMemoryMode(pid);
 
   if (IsMemoryDBActive(pid, write)) {
     if (uid_ > REGULAR_USER)
-      db_path_list.emplace_back(std::make_pair(parser_memory_db_path_, uid_));
+      db_path_list.emplace_back(std::make_pair(user_parser_memdb_path_, uid_));
+
     db_path_list.emplace_back(
-        std::make_pair(global_parser_memory_db_path, GetGlobalUID()));
+        std::make_pair(global_parser_memdb_path, GetGlobalUID()));
   } else {
     if (uid_ > REGULAR_USER)
-      db_path_list.emplace_back(std::make_pair(parser_file_db_path_, uid_));
+      db_path_list.emplace_back(std::make_pair(user_parser_filedb_path_, uid_));
+
     db_path_list.emplace_back(
-        std::make_pair(global_parser_file_db_path_, GetGlobalUID()));
+        std::make_pair(global_parser_filedb_path_, GetGlobalUID()));
   }
 
   if (db_path_list.size() == 1) {
@@ -179,16 +183,16 @@ std::vector<std::pair<std::string, uid_t>> DBHandleProvider::GetParserDBPath(
 
 std::string DBHandleProvider::GetCertDBPath(pid_t pid, bool write) {
   std::unique_lock<std::recursive_mutex> u(lock_);
-  if (is_memory_ != is_memory_global_)
-    is_memory_global_ ? SetMemoryMode(pid) : UnsetMemoryMode(pid);
+  if (is_user_memdb_set_ != is_global_memdb_set_)
+    is_global_memdb_set_ ? SetMemoryMode(pid) : UnsetMemoryMode(pid);
 
   if (IsMemoryDBActive(pid, write))
-    return cert_memory_db_path;
+    return cert_memdb_path;
   else
-    return cert_file_db_path_;
+    return cert_filedb_path_;
 }
 
-sqlite3* DBHandleProvider::GetMemoryDBHandle(const std::string& filedb_path,
+sqlite3* DBHandleProvider::CreateMemoryDBHandle(const std::string& filedb_path,
                                              const std::string& memorydb_path) {
   sqlite3* memorydb = nullptr;
   sqlite3* filedb = nullptr;
@@ -224,49 +228,51 @@ sqlite3* DBHandleProvider::GetMemoryDBHandle(const std::string& filedb_path,
 
 void DBHandleProvider::SetMemoryMode(pid_t pid) {
   std::unique_lock<std::recursive_mutex> u(lock_);
-  if (is_memory_global_ && is_memory_)
+  if (is_global_memdb_set_ && is_user_memdb_set_)
     return;
 
   sqlite3* parser_db =
-      GetMemoryDBHandle(parser_file_db_path_, parser_memory_db_path_);
+      CreateMemoryDBHandle(user_parser_filedb_path_, user_parser_memdb_path_);
   if (parser_db != nullptr)
-    parser_memory_db_handle_.reset(parser_db);
+    parser_memdb_handle_.reset(parser_db);
 
-  if (is_memory_ == is_memory_global_) {
-    sqlite3* global_parser_file_db = GetMemoryDBHandle(
-        global_parser_file_db_path_, global_parser_memory_db_path);
+  if (is_user_memdb_set_ == is_global_memdb_set_) {
+    sqlite3* global_parser_file_db = CreateMemoryDBHandle(
+        global_parser_filedb_path_, global_parser_memdb_path);
     if (global_parser_file_db)
-      global_parser_memory_db_handle_.reset(global_parser_file_db);
+      global_parser_memdb_handle_.reset(global_parser_file_db);
+
     sqlite3* cert_db =
-        GetMemoryDBHandle(cert_file_db_path_, cert_memory_db_path);
+        CreateMemoryDBHandle(cert_filedb_path_, cert_memdb_path);
     if (cert_db != nullptr)
-      cert_memory_db_handle_.reset(cert_db);
+      cert_memdb_handle_.reset(cert_db);
 
     InsertPID(pid);
   }
 
-  is_memory_ = true;
-  is_memory_global_ = true;
+  is_user_memdb_set_ = true;
+  is_global_memdb_set_ = true;
   LOG(DEBUG) << "Set Memory mode : Memory";
 }
 
 void DBHandleProvider::UnsetMemoryMode(pid_t pid) {
   std::unique_lock<std::recursive_mutex> u(lock_);
-  if (!is_memory_global_ && !is_memory_)
+  if (!is_global_memdb_set_ && !is_user_memdb_set_)
     return;
 
-  auto lock = CacheFlag::GetWriterLock();
-  CacheFlag::SetStatus(CacheFlag::Status::PREPARING);
-
-  parser_memory_db_handle_.reset(nullptr);
-  cert_memory_db_handle_.reset(nullptr);
-  global_parser_memory_db_handle_.reset(nullptr);
+  parser_memdb_handle_.reset(nullptr);
+  cert_memdb_handle_.reset(nullptr);
+  global_parser_memdb_handle_.reset(nullptr);
 
   if (!ErasePID(pid))
     LOG(ERROR) << "Given pid is not exists in pid list : " << pid;
 
-  is_memory_ = false;
-  is_memory_global_ = false;
+  is_user_memdb_set_ = false;
+  is_global_memdb_set_ = false;
+
+  auto lock = CacheFlag::GetWriterLock();
+  CacheFlag::SetStatus(CacheFlag::Status::PREPARING);
+
   pkg_map_.clear();
   app_map_.clear();
   CacheFlag::SetStatus(CacheFlag::Status::UNPREPARED);
@@ -275,7 +281,16 @@ void DBHandleProvider::UnsetMemoryMode(pid_t pid) {
 
 bool DBHandleProvider::IsMemoryDBActive(pid_t pid, bool write) {
   std::unique_lock<std::mutex> u(pid_list_lock_);
-  return (is_memory_ && pid_list_.find(pid) == pid_list_.end() && !write);
+  if (!is_user_memdb_set_)
+    return false;
+
+  if (write)
+    return false;
+
+  if (writer_pid_list_.find(pid) != writer_pid_list_.end())
+    return false;
+
+  return true;
 }
 
 void DBHandleProvider::TrimCache() {
@@ -297,7 +312,7 @@ void DBHandleProvider::ReleaseCache() {
 
 bool DBHandleProvider::IsWriter(pid_t pid) {
   std::unique_lock<std::mutex> u(pid_list_lock_);
-  return pid_list_.find(pid) != pid_list_.end();
+  return writer_pid_list_.find(pid) != writer_pid_list_.end();
 }
 
 int DBHandleProvider::UpdateCache(sqlite3* db, pid_t pid, uid_t uid, bool write,
@@ -445,13 +460,13 @@ void DBHandleProvider::AddApplication(std::string app, application_x* info) {
 void DBHandleProvider::InsertPID(pid_t pid) {
   std::unique_lock<std::mutex> u(pid_list_lock_);
 
-  pid_list_.insert(pid);
+  writer_pid_list_.insert(pid);
 }
 
 bool DBHandleProvider::ErasePID(pid_t pid) {
   std::unique_lock<std::mutex> u(pid_list_lock_);
 
-  return pid_list_.erase(pid) == 1;
+  return writer_pid_list_.erase(pid) == 1;
 }
 
 }  // namespace database

diff --git a/src/server/database/db_handle_provider.hh b/src/server/database/db_handle_provider.hh
index 1a7dff6..9631cf5 100644 (file)
--- a/src/server/database/db_handle_provider.hh
+++ b/src/server/database/db_handle_provider.hh
@@ -63,7 +63,7 @@ class EXPORT_API DBHandleProvider {
 
  private:
   explicit DBHandleProvider(uid_t uid);
-  sqlite3* GetMemoryDBHandle(const std::string& filedb_path,
+  sqlite3* CreateMemoryDBHandle(const std::string& filedb_path,
                              const std::string& memorydb_path);
   bool IsMemoryDBActive(pid_t pid, bool write);
   void ReleaseCache();
@@ -73,24 +73,25 @@ class EXPORT_API DBHandleProvider {
   bool ErasePID(pid_t pid);
 
  private:
-  static std::unordered_map<uid_t, std::unique_ptr<DBHandleProvider>> provider_;
-  static bool is_memory_global_;
+  static std::unordered_map<uid_t,
+        std::unique_ptr<DBHandleProvider>> provider_map_;
+  static bool is_global_memdb_set_;
   static std::unique_ptr<sqlite3, decltype(sqlite3_close_v2)*>
-      global_parser_memory_db_handle_;
+      global_parser_memdb_handle_;
   static std::unique_ptr<sqlite3, decltype(sqlite3_close_v2)*>
-      cert_memory_db_handle_;
-  static std::string global_parser_file_db_path_;
-  static std::string cert_file_db_path_;
-  static std::unordered_set<pid_t> pid_list_;
+      cert_memdb_handle_;
+  static std::string global_parser_filedb_path_;
+  static std::string cert_filedb_path_;
+  static std::unordered_set<pid_t> writer_pid_list_;
   static std::recursive_mutex lock_;
   static std::mutex pid_list_lock_;
 
   uid_t uid_;
-  bool is_memory_;
+  bool is_user_memdb_set_;
   std::unique_ptr<sqlite3, decltype(sqlite3_close_v2)*>
-      parser_memory_db_handle_;
-  std::string parser_memory_db_path_;
-  std::string parser_file_db_path_;
+      parser_memdb_handle_;
+  std::string user_parser_memdb_path_;
+  std::string user_parser_filedb_path_;
   bool released_ = true;
   std::unordered_map<std::string, std::vector<std::shared_ptr<package_x>>>
       pkg_map_;

diff --git a/src/server/request_handler/get_appinfo_request_handler.cc b/src/server/request_handler/get_appinfo_request_handler.cc
index f558a9e..e26069f 100644 (file)
--- a/src/server/request_handler/get_appinfo_request_handler.cc
+++ b/src/server/request_handler/get_appinfo_request_handler.cc
@@ -18,10 +18,6 @@ namespace psd = pkgmgr_server::database;
 namespace pkgmgr_server {
 namespace request_handler {
 
-GetAppinfoRequestHandler::GetAppinfoRequestHandler(bool cache)
-    : cache_(cache) {
-}
-
 bool GetAppinfoRequestHandler::HandleRequest(unsigned char* data, int size,
                                              const std::string& locale) {
   auto abstract_parcel =

diff --git a/src/server/request_handler/get_appinfo_request_handler.hh b/src/server/request_handler/get_appinfo_request_handler.hh
index 71c574c..0110e36 100644 (file)
--- a/src/server/request_handler/get_appinfo_request_handler.hh
+++ b/src/server/request_handler/get_appinfo_request_handler.hh
@@ -19,7 +19,6 @@ namespace request_handler {
 
 class EXPORT_API GetAppinfoRequestHandler : public AbstractRequestHandler {
  public:
-  explicit GetAppinfoRequestHandler(bool cache = false);
   bool HandleRequest(unsigned char *data, int size,
       const std::string &locale) override;
 
@@ -27,7 +26,6 @@ class EXPORT_API GetAppinfoRequestHandler : public AbstractRequestHandler {
 
  private:
   std::shared_ptr<pkgmgr_common::parcel::AppInfoParcelable> result_;
-  bool cache_;
 };
 
 }  // namespace request_handler

diff --git a/src/server/request_handler/get_pkginfo_request_handler.cc b/src/server/request_handler/get_pkginfo_request_handler.cc
index d99b6f8..acd9f67 100644 (file)
--- a/src/server/request_handler/get_pkginfo_request_handler.cc
+++ b/src/server/request_handler/get_pkginfo_request_handler.cc
@@ -19,10 +19,6 @@ namespace psd = pkgmgr_server::database;
 namespace pkgmgr_server {
 namespace request_handler {
 
-GetPkginfoRequestHandler::GetPkginfoRequestHandler(bool cache)
-    : cache_(cache) {
-}
-
 bool GetPkginfoRequestHandler::HandleRequest(unsigned char* data, int size,
     const std::string& locale) {
   auto abstract_parcel =

diff --git a/src/server/request_handler/get_pkginfo_request_handler.hh b/src/server/request_handler/get_pkginfo_request_handler.hh
index c567326..18c6b28 100644 (file)
--- a/src/server/request_handler/get_pkginfo_request_handler.hh
+++ b/src/server/request_handler/get_pkginfo_request_handler.hh
@@ -19,7 +19,6 @@ namespace request_handler {
 
 class EXPORT_API GetPkginfoRequestHandler : public AbstractRequestHandler {
  public:
-  explicit GetPkginfoRequestHandler(bool cache = false);
   bool HandleRequest(unsigned char* data, int size,
       const std::string& locale) override;
 

diff --git a/src/server/worker_thread.cc b/src/server/worker_thread.cc
index a1b0387..08f2929 100644 (file)
--- a/src/server/worker_thread.cc
+++ b/src/server/worker_thread.cc
@@ -75,9 +75,9 @@ void WorkerThread::Run() {
   std::unique_ptr<request_handler::AbstractRequestHandler>
       handler[pkgmgr_common::ReqType::MAX];
   handler[pkgmgr_common::ReqType::GET_PKG_INFO].reset(
-      new request_handler::GetPkginfoRequestHandler(true));
+      new request_handler::GetPkginfoRequestHandler());
   handler[pkgmgr_common::ReqType::GET_APP_INFO].reset(
-      new request_handler::GetAppinfoRequestHandler(true));
+      new request_handler::GetAppinfoRequestHandler());
   handler[pkgmgr_common::ReqType::SET_PKG_INFO].reset(
       new request_handler::SetPkginfoRequestHandler());
   handler[pkgmgr_common::ReqType::SET_CERT_INFO].reset(
@@ -113,13 +113,8 @@ void WorkerThread::Run() {
             || type >= pkgmgr_common::ReqType::MAX) {
       LOG(ERROR) << "Request type is invalid: " << static_cast<int>(type)
           << ",  pid:" << req->GetSenderPID();
+      SendError(req);
 
-      pkgmgr_common::parcel::AbstractParcelable parcelable(
-          0, pkgmgr_common::parcel::ParcelableType::Unknown, PMINFO_R_ERROR);
-      tizen_base::Parcel p;
-      p.WriteParcelable(parcelable);
-      std::vector<uint8_t> raw = p.GetRaw();
-      req->SendData(&raw[0], raw.size());
       continue;
     }
 
@@ -131,21 +126,13 @@ void WorkerThread::Run() {
     } catch (const std::exception& err) {
       LOG(ERROR) << "Exception occurred: " << err.what()
           << ", pid: " << req->GetSenderPID();
-      pkgmgr_common::parcel::AbstractParcelable parcelable(
-          0, pkgmgr_common::parcel::ParcelableType::Unknown, PMINFO_R_ERROR);
-      tizen_base::Parcel p;
-      p.WriteParcelable(parcelable);
-      std::vector<uint8_t> raw = p.GetRaw();
-      req->SendData(&raw[0], raw.size());
+      SendError(req);
+
       continue;
     } catch (...) {
       LOG(ERROR) << "Exception occurred pid: " << req->GetSenderPID();
-      pkgmgr_common::parcel::AbstractParcelable parcelable(
-          0, pkgmgr_common::parcel::ParcelableType::Unknown, PMINFO_R_ERROR);
-      tizen_base::Parcel p;
-      p.WriteParcelable(parcelable);
-      std::vector<uint8_t> raw = p.GetRaw();
-      req->SendData(&raw[0], raw.size());
+      SendError(req);
+
       continue;
     }
 
@@ -197,4 +184,13 @@ void WorkerThread::SetLocale(std::string locale) {
   locale_.SetObject(std::move(locale));
 }
 
+void WorkerThread::SendError(std::shared_ptr<PkgRequest> req) {
+  pkgmgr_common::parcel::AbstractParcelable parcelable(
+      0, pkgmgr_common::parcel::ParcelableType::Unknown, PMINFO_R_ERROR);
+  tizen_base::Parcel p;
+  p.WriteParcelable(parcelable);
+  std::vector<uint8_t> raw = p.GetRaw();
+  req->SendData(&raw[0], raw.size());
+}
+
 }  // namespace pkgmgr_server

diff --git a/src/server/worker_thread.hh b/src/server/worker_thread.hh
index 813fdbb..809ca88 100644 (file)
--- a/src/server/worker_thread.hh
+++ b/src/server/worker_thread.hh
@@ -45,6 +45,7 @@ class EXPORT_API WorkerThread {
 
  private:
   void Run();
+  void SendError(std::shared_ptr<PkgRequest> req);
   static void SetMemoryTrimTimer();
   static gboolean TrimMemory(void* data);
   std::shared_ptr<PkgRequest> PopQueue();