Improve code readability.

- Add some empty lines for readability.
- Reduce indentation depth.
- Extract codes into function.

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

diff --git a/src/pkg_upgrade/src/backend_invoker.cc b/src/pkg_upgrade/src/backend_invoker.cc
index 1308bad..f3a3e14 100644 (file)
--- a/src/pkg_upgrade/src/backend_invoker.cc
+++ b/src/pkg_upgrade/src/backend_invoker.cc
@@ -105,7 +105,6 @@ void BackendInvoker::SetRemovable(bool removable) {
     parameters_.push_back(kNoRemoveOptStr);
 }
 
-
 int BackendInvoker::Run() const {
   const char* param[parameters_.size() + 1] = { nullptr, };
 

diff --git a/src/pkg_upgrade/src/file_logbackend.cc b/src/pkg_upgrade/src/file_logbackend.cc
index 6e36fe3..f386c65 100644 (file)
--- a/src/pkg_upgrade/src/file_logbackend.cc
+++ b/src/pkg_upgrade/src/file_logbackend.cc
@@ -47,10 +47,10 @@ void FileLogBackend::WriteLogToFile() {
   if (file_name_.empty())
     return;
 
-  int size = GetFileSize(file_name_);
-  if (size > rotation_size_)
+  if (GetFileSize(file_name_) > rotation_size_) {
     if (!Rotate())
       return;
+  }
 
   std::ofstream ofs(file_name_.c_str(), std::ios::app);
   ofs << log_stream_->str();
@@ -64,6 +64,7 @@ void FileLogBackend::WriteLogToFile() {
 bool FileLogBackend::Rotate() {
   for (int i = max_rotation_; i > 0; i--) {
     std::string old_log = file_name_ + "." + std::to_string(i);
+
     // the oldest log will be removed
     if (i == max_rotation_) {
       if (std::remove(old_log.c_str()) != 0)
@@ -74,6 +75,7 @@ bool FileLogBackend::Rotate() {
         return false;
     }
   }
+
   std::string new_log = file_name_ + ".1";
   if (std::rename(file_name_.c_str(), new_log.c_str()) != 0)
     return false;
@@ -95,10 +97,12 @@ std::string FileLogBackend::GetTimeStamp() {
   struct tm gmt;
   if (!gmtime_r(&seconds, &gmt))
     return "|";
+
   int32_t miliseconds = ts.tv_nsec / 1000000;
 
   char buf[32];
   strftime(buf, sizeof(buf), "%Y%m%d.%H%M%S", &gmt);
+
   char timestamp[64];
   snprintf(timestamp, sizeof(timestamp), "%s.%03dUTC|", buf, miliseconds);
 

diff --git a/src/pkg_upgrade/src/pkg_finder.cc b/src/pkg_upgrade/src/pkg_finder.cc
index 8763431..02e541c 100644 (file)
--- a/src/pkg_upgrade/src/pkg_finder.cc
+++ b/src/pkg_upgrade/src/pkg_finder.cc
@@ -39,7 +39,6 @@
 #define PKGMGR_FOTA_PATH        tzplatform_mkpath(TZ_SYS_GLOBALUSER_DATA, \
     "pkgmgr/fota")
 
-
 using std::string;
 
 namespace {
@@ -52,7 +51,6 @@ constexpr char kTokenTypeStr[] = "type=";
 constexpr char kTokenPkgidStr[] = "package=";
 constexpr char kTokenVersionStr[] = "version=";
 constexpr char kTokenRemoveStr[] = "removable=";
-
 constexpr char kDefaultVersionStr[] = "0.0.1";
 constexpr char kDefaultPkgTypeStr[] = "tpk";
 
@@ -64,11 +62,11 @@ PkgFinder::PkgFinder() {
   xmlInitParser();
   manifest_dir_ = USR_MANIFEST_DIRECTORY;
   preload_rw_list_path_ = PRELOAD_RW_LIST_FILE;
+
   if (access(kOptZipFile, F_OK) != 0)
     return;
 
-  int ret = UnzipFileOnlyToPath(ALL_PRELOAD_RW_PKG_LIST, PKGMGR_FOTA_PATH);
-  if (ret != 0) {
+  if (UnzipFileOnlyToPath(ALL_PRELOAD_RW_PKG_LIST, PKGMGR_FOTA_PATH) != 0) {
       LOG(ERROR) << "Failed to unzip file from backup";
       return;
   }
@@ -82,6 +80,7 @@ int PkgFinder::UnzipFileOnlyToPath(const char* dest_path,
     const char* unzip_to) {
   const char* unzip_argv[] = { "/usr/bin/unzip", "-joXqq",
       kOptZipFile, dest_path, "-d", unzip_to, nullptr };
+
   return BackendInvoker::XSystem(unzip_argv);
 }
 
@@ -119,23 +118,20 @@ void PkgFinder::SetPreloadRwListPath(std::string path) {
 
 int PkgFinder::PkgidListCb(const pkgmgrinfo_pkginfo_h handle, void* user_data) {
   char* pkgid = nullptr;
-  int ret = -1;
-  ret = pkgmgrinfo_pkginfo_get_pkgid(handle, &pkgid);
-  if (ret < 0)
+  if (pkgmgrinfo_pkginfo_get_pkgid(handle, &pkgid) != PMINFO_R_OK)
     return 0;
 
   char* version = nullptr;
-  ret = pkgmgrinfo_pkginfo_get_version(handle, &version);
-  if (ret < 0)
+  if (pkgmgrinfo_pkginfo_get_version(handle, &version) != PMINFO_R_OK)
     return 0;
 
   char* type = nullptr;
-  ret = pkgmgrinfo_pkginfo_get_type(handle, &type);
-  if (ret < 0)
+  if (pkgmgrinfo_pkginfo_get_type(handle, &type) != PMINFO_R_OK)
     return 0;
 
   PkgFinder* finder = static_cast<PkgFinder*>(user_data);
   finder->old_pkgs_.emplace_back(pkgid, version, type, finder->read_only_);
+
   return 0;
 }
 
@@ -166,8 +162,8 @@ int PkgFinder::FindPreloadPkgidFromDb(bool read_only) {
     LOG(ERROR) << "pkgmgrinfo_pkginfo_filter_add_bool failed " << ret;
     return -1;
   }
-
   read_only_ = read_only;
+
   ret = pkgmgrinfo_pkginfo_filter_foreach_pkginfo(handle_auto.get(),
     PkgidListCb, this);
   if (ret != PMINFO_R_OK) {
@@ -190,12 +186,12 @@ int PkgFinder::FindPreloadPkgidFromXml(
   }
 
   while ((entry = readdir(dir)) != nullptr) {
-    if (entry->d_name[0] == '.') continue;
+    if (entry->d_name[0] == '.')
+      continue;
 
     string manifest = GetValidManifest(entry->d_name);
-    if (manifest.empty()) {
+    if (manifest.empty())
       continue;
-    }
 
     string buf = xml_directory + "/" + manifest;
     string pkgid = FindInfoFromXml(buf, "package");
@@ -233,41 +229,41 @@ string PkgFinder::FindInfoFromXml(const string& manifest,
 
   reader = xmlReaderForFile(manifest.c_str(), nullptr, 0);
 
-  if (reader) {
-    if (MoveToChildElement(reader, -1)) {
-      node = xmlTextReaderConstName(reader);
-      if (!node) {
-        LOG(ERROR) << "xmlTextReaderConstName value is NULL";
-        goto end;
-      }
-
-      if (!strcmp(ASCII(node), "manifest")) {
-        tmp = xmlTextReaderGetAttribute(reader,
-          XMLCHAR(find_info.c_str()));
-        if (tmp) {
-          info_val = ASCII(tmp);
-          free(tmp);
-          tmp = nullptr;
-        }
-      } else {
-        LOG(ERROR) << "Manifest Node is not found";
-      }
-    }
-  } else {
+  if (!reader) {
     LOG(ERROR) << "xmlReaderForFile value is NULL";
+    return "";
+  }
+  std::unique_ptr<std::remove_pointer<xmlTextReaderPtr>::type,
+      decltype(xmlFreeTextReader)*> reader_auto(
+          reader, xmlFreeTextReader);
+
+  if (!MoveToChildElement(reader_auto.get(), -1))
+    return {};
+
+  node = xmlTextReaderConstName(reader_auto.get());
+  if (!node) {
+    LOG(ERROR) << "xmlTextReaderConstName value is NULL";
+    return {};
   }
 
-end:
-  if (reader)
-    xmlFreeTextReader(reader);
+  if (!strcmp(ASCII(node), "manifest")) {
+    tmp = xmlTextReaderGetAttribute(reader_auto.get(),
+      XMLCHAR(find_info.c_str()));
+    if (tmp) {
+      info_val = ASCII(tmp);
+      free(tmp);
+    }
+  } else {
+    LOG(ERROR) << "Manifest Node is not found";
+  }
 
   return info_val;
 }
 
-int PkgFinder::MoveToChildElement(xmlTextReaderPtr reader,
-    int depth) {
+int PkgFinder::MoveToChildElement(xmlTextReaderPtr reader, int depth) {
   int ret = xmlTextReaderRead(reader);
   int cur = xmlTextReaderDepth(reader);
+
   while (ret == 1) {
     switch (xmlTextReaderNodeType(reader)) {
     case XML_READER_TYPE_ELEMENT:
@@ -291,12 +287,13 @@ int PkgFinder::MoveToChildElement(xmlTextReaderPtr reader,
     ret = xmlTextReaderRead(reader);
     cur = xmlTextReaderDepth(reader);
   }
+
   return ret;
 }
 
 int PkgFinder::FindPreloadPkgidFromFile() {
   char buf[kBufSize] = {0};
-  FILE *fp = nullptr;
+  FILE *fp;
 
   fp = fopen(preload_rw_list_path_.c_str(), "r");
   if (fp == nullptr) {

diff --git a/src/pkg_upgrade/src/pkg_upgrader_factory.cc b/src/pkg_upgrade/src/pkg_upgrader_factory.cc
index f0dffbb..3e3499b 100644 (file)
--- a/src/pkg_upgrade/src/pkg_upgrader_factory.cc
+++ b/src/pkg_upgrade/src/pkg_upgrader_factory.cc
@@ -29,6 +29,35 @@ using std::list;
 using std::string;
 using std::unique_ptr;
 
+namespace {
+
+enum class UpdateType {
+  kRo2Ro,
+  kRw2Rw,
+  kRw2Ro,
+  kRo2Rw,
+  kUnknown
+};
+
+UpdateType GetUpdateType(
+    common_fota::PkgContext old_pkg, common_fota::PkgContext new_pkg) {
+  if (old_pkg.IsReadOnly() && new_pkg.IsReadOnly())
+    return UpdateType::kRo2Ro;
+
+  if (!old_pkg.IsReadOnly() && !new_pkg.IsReadOnly())
+    return UpdateType::kRw2Rw;
+
+  if (!old_pkg.IsReadOnly() && new_pkg.IsReadOnly())
+    return UpdateType::kRw2Ro;
+
+  if (old_pkg.IsReadOnly() && !new_pkg.IsReadOnly())
+    return UpdateType::kRo2Rw;
+
+  return UpdateType::kUnknown;
+}
+
+}  // namespace
+
 namespace common_fota {
 
 list<unique_ptr<PkgUpgrader>> PkgUpgraderFactory::MakeList(PkgFinder* finder) {
@@ -47,27 +76,29 @@ list<unique_ptr<PkgUpgrader>> PkgUpgraderFactory::Merge(
     const auto* old_pkg = FindPkgById(old_pkgs, new_pkg.GetId());
     if (old_pkg != nullptr) {
       // UPDATE
-      if (old_pkg->IsReadOnly() && new_pkg.IsReadOnly()) {
-        // RO to RO
+      UpdateType update_type = GetUpdateType(*old_pkg, new_pkg);
+
+      switch(update_type) {
+      case UpdateType::kRo2Ro:
         pkgs.emplace_back(std::make_unique<RoUpgrader>(
             std::make_unique<SimpleUpgrader>(*old_pkg, PkgOperation::COMPLEX),
             std::make_unique<SimpleUpgrader>(new_pkg, PkgOperation::UPDATE)));
-      } else if (!old_pkg->IsReadOnly() && !new_pkg.IsReadOnly()) {
-        // RW to RW
+        break;
+      case UpdateType::kRw2Rw:
         pkgs.emplace_back(std::make_unique<RwUpgrader>(
             std::make_unique<SimpleUpgrader>(*old_pkg, PkgOperation::COMPLEX),
             std::make_unique<SimpleUpgrader>(new_pkg, PkgOperation::UPDATE)));
-      } else if (!old_pkg->IsReadOnly() && new_pkg.IsReadOnly()) {
-        // RW to RO
+        break;
+      case UpdateType::kRw2Ro:
         pkgs.emplace_back(std::make_unique<Rw2RoUpgrader>(
-            std::make_unique<RwUpgrader>(nullptr,
-                std::make_unique<SimpleUpgrader>(*old_pkg,
-                    PkgOperation::UNINSTALL_KEEP_RW_DATA)),
-            std::make_unique<RoUpgrader>(nullptr,
-                std::make_unique<SimpleUpgrader>(new_pkg,
-                    PkgOperation::INSTALL))));
-      } else if (old_pkg->IsReadOnly() && !new_pkg.IsReadOnly()) {
-        // RO to RW
+              std::make_unique<RwUpgrader>(nullptr,
+                  std::make_unique<SimpleUpgrader>(*old_pkg,
+                      PkgOperation::UNINSTALL_KEEP_RW_DATA)),
+              std::make_unique<RoUpgrader>(nullptr,
+                  std::make_unique<SimpleUpgrader>(new_pkg,
+                      PkgOperation::INSTALL))));
+        break;
+      case UpdateType::kRo2Rw:
         pkgs.emplace_back(std::make_unique<Ro2RwUpgrader>(
             std::make_unique<RoUpgrader>(nullptr,
                 std::make_unique<SimpleUpgrader>(*old_pkg,
@@ -75,6 +106,9 @@ list<unique_ptr<PkgUpgrader>> PkgUpgraderFactory::Merge(
             std::make_unique<RwUpgrader>(nullptr,
                 std::make_unique<SimpleUpgrader>(new_pkg,
                     PkgOperation::INSTALL))));
+        break;
+      default:
+        break;
       }
     } else {
       // INSTALL