Fix minor coding rules

- Fix some minor coding rule.
- Fix storage requirement calculation logic to reduce unnecessary if statement.
- Remove unnecessary dlerror() call.
- Integrate some if statements.
- Remove implementation of undo function info header.
- Apply return early pattern.
- Remove unnecessary constructor implementation.

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

diff --git a/src/common/app2ext_dynamic_service.cc b/src/common/app2ext_dynamic_service.cc
index be490b7adba967c46d0a9c1e709d7de1fdc8f9aa..b29d71fd68787eb65c43c4ba298d6d3d15a94bd7 100644 (file)
--- a/src/common/app2ext_dynamic_service.cc
+++ b/src/common/app2ext_dynamic_service.cc
@@ -29,7 +29,6 @@ App2ExtDynamicService::App2ExtDynamicService()
     LOG(WARNING) << "Cannot open handle: " << dlerror();
     return;
   }
-  dlerror();
 }
 
 App2ExtDynamicService::~App2ExtDynamicService() {
@@ -84,15 +83,12 @@ bool App2ExtDynamicService::ProperlyLoaded() {
 }
 
 std::shared_ptr<app2ext_handle> App2ExtDynamicService::getInterfaceHandle() {
-  if (app2ext_handler)
-    return app2ext_handler;
-  if (ProperlyLoaded()) {
+  if (!app2ext_handler && ProperlyLoaded()) {
     app2ext_handler = std::shared_ptr<app2ext_handle>(InitLibrary(),
         std::bind(&App2ExtDynamicService::DeinitLibrary,
                   this, std::placeholders::_1));
-    if (!app2ext_handler) {
+    if (!app2ext_handler)
         LOG(ERROR) "app2ext_init() failed.";
-    }
   }
   return app2ext_handler;
 }

diff --git a/src/common/external_storage.cc b/src/common/external_storage.cc
index 5b016858a3301f23694b75f00d1e358f796e8be3..68be0112819eb488c59b4d5567783cd4649f3cef 100644 (file)
--- a/src/common/external_storage.cc
+++ b/src/common/external_storage.cc
@@ -51,11 +51,10 @@ ExternalStorage::ExternalStorage(RequestType type,
       uid_(uid),
       move_type_(-1),
       handle_(nullptr) {
-  if (package_type_ == kWgtType) {
+  if (package_type_ == kWgtType)
     external_dirs_.push_back(kExternalDirForWgt);
-  } else {
+  else
     external_dirs_ = kExternalDirsForTpk;
-  }
 }
 
 ExternalStorage::ExternalStorage(RequestType type,
@@ -68,11 +67,11 @@ ExternalStorage::ExternalStorage(RequestType type,
       application_root_(application_root),
       uid_(uid),
       handle_(nullptr) {
-  if (package_type_ == kWgtType) {
+  if (package_type_ == kWgtType)
     external_dirs_.push_back(kExternalDirForWgt);
-  } else {
+  else
     external_dirs_ = kExternalDirsForTpk;
-  }
+
   if (is_external_move)
     move_type_ = APP2EXT_MOVE_TO_EXT;
   else
@@ -150,14 +149,11 @@ bool ExternalStorage::Initialize(
       }
     } else {
       // for wgt whole content of package goes to res/
-      external_size =
+      external_size +=
           SizeInMB(GetDirectorySize(space_requirement));
     }
   }
 
-  if (external_size == 0)
-    external_size = 1;
-
   handle_ = service.getInterfaceHandle();
   if (!handle_) {
     LOG(ERROR) << "app2ext_init() failed";

diff --git a/src/common/step/backup/step_backup_icons.cc b/src/common/step/backup/step_backup_icons.cc
index 9c726a90d67a41a8601d0edcc2d0834163ddafef..6b0b13e86f6f108c5ffa10fadc4ab0133f2714c0 100644 (file)
--- a/src/common/step/backup/step_backup_icons.cc
+++ b/src/common/step/backup/step_backup_icons.cc
@@ -34,14 +34,14 @@ Step::Status StepBackupIcons::process() {
       continue;
     for (application_x* app : GListRange<application_x*>(
         context_->old_manifest_data.get()->application)) {
-      if (app->icon) {
-        bf::path filename = iter->path().filename();
-        filename.replace_extension();
-        std::string id = filename.string();
-        if (id == app->appid) {
-          bf::path icon_backup = GetBackupPathForIconFile(iter->path());
-          icons_.emplace_back(iter->path(), icon_backup);
-        }
+      if (!app->icon)
+        continue;
+      bf::path filename = iter->path().filename();
+      filename.replace_extension();
+      std::string id = filename.string();
+      if (id == app->appid) {
+        bf::path icon_backup = GetBackupPathForIconFile(iter->path());
+        icons_.emplace_back(iter->path(), icon_backup);
       }
     }
   }

diff --git a/src/common/step/filesystem/step_copy_storage_directories.cc b/src/common/step/filesystem/step_copy_storage_directories.cc
index 3781d577af90c23bd2078b32672284958e411b8a..37b2903576aaa4412c51576c17987d7d32ed9f8f 100644 (file)
--- a/src/common/step/filesystem/step_copy_storage_directories.cc
+++ b/src/common/step/filesystem/step_copy_storage_directories.cc
@@ -75,10 +75,6 @@ common_installer::Step::Status StepCopyStorageDirectories::process() {
   return Status::OK;
 }
 
-common_installer::Step::Status StepCopyStorageDirectories::undo() {
-  return Status::OK;
-}
-
 bool StepCopyStorageDirectories::CacheDir() {
   bs::error_code error_code;
   bf::path cache_path = context_->GetPkgPath() / kCache;

diff --git a/src/common/step/filesystem/step_copy_storage_directories.h b/src/common/step/filesystem/step_copy_storage_directories.h
index e33bb4b89cbd5aded3c77a20dd712ca4b65b1038..7954a285002898506aefa409aa9f87a9fd580dfc 100644 (file)
--- a/src/common/step/filesystem/step_copy_storage_directories.h
+++ b/src/common/step/filesystem/step_copy_storage_directories.h
@@ -30,13 +30,7 @@ class StepCopyStorageDirectories : public common_installer::Step {
    */
   Status process() override;
   Status clean() override { return Status::OK; }
-
-  /**
-   * \brief restores original content of data and shared dirs
-   *
-   * \return Status::OK if success, Status::ERROR otherwise
-   */
-  Status undo() override;
+  Status undo() override { return Status::OK; }
 
   /**
    * \brief checks if needed paths/data are provided

diff --git a/src/common/step/filesystem/step_create_icons.cc b/src/common/step/filesystem/step_create_icons.cc
index 06ceeefd76657c345c20984b09066601e4c97c8c..391f20e010ab15f70a446335d181a6315f8a80f1 100644 (file)
--- a/src/common/step/filesystem/step_create_icons.cc
+++ b/src/common/step/filesystem/step_create_icons.cc
@@ -18,9 +18,9 @@ namespace common_installer {
 namespace filesystem {
 
 Step::Status StepCreateIcons::undo() {
-  for (auto& icon : icons_) {
+  for (auto& icon : icons_)
     RemoveAll(icon);
-  }
+
   return Status::OK;
 }
 
@@ -45,25 +45,27 @@ Step::Status StepCreateIcons::process() {
       GListRange<application_x*>(context_->manifest_data.get()->application)) {
     // TODO(t.iwanek): this ignores icon locale as well in same way as other
     // steps -> icons should be localized
-    if (app->icon) {
-      icon_x* icon = reinterpret_cast<icon_x*>(app->icon->data);
-      bf::path source(icon->text);
-      if (bf::exists(source)) {
-        bf::path destination_path = destination / app->appid;
-        if (source.has_extension())
-          destination_path += source.extension();
-        else
-          destination_path += ".png";
-        bf::copy_file(source, destination_path,
-                        bf::copy_option::overwrite_if_exists, error);
-        if (error) {
-          LOG(ERROR) << "Cannot create package icon: " << destination_path
-                     << " , error: " << error;
-          return Status::ICON_ERROR;
-        }
-        icons_.push_back(destination_path);
-      }
+    if (!app->icon)
+      continue;
+
+    icon_x* icon = reinterpret_cast<icon_x*>(app->icon->data);
+    bf::path source(icon->text);
+    if (!bf::exists(source))
+      continue;
+
+    bf::path destination_path = destination / app->appid;
+    if (source.has_extension())
+      destination_path += source.extension();
+    else
+      destination_path += ".png";
+    bf::copy_file(source, destination_path,
+                  bf::copy_option::overwrite_if_exists, error);
+    if (error) {
+      LOG(ERROR) << "Cannot create package icon: " << destination_path
+                 << " , error: " << error;
+      return Status::ICON_ERROR;
     }
+    icons_.push_back(destination_path);
   }
   LOG(DEBUG) << "Icon files created";
   return Status::OK;

diff --git a/src/common/step/filesystem/step_optional_acquire_external_storage.cc b/src/common/step/filesystem/step_optional_acquire_external_storage.cc
index 9bc4356e702366822729861c9a3735b650b96458..1b47b65581cb4823aece88bb0f4a6e10aff836a1 100644 (file)
--- a/src/common/step/filesystem/step_optional_acquire_external_storage.cc
+++ b/src/common/step/filesystem/step_optional_acquire_external_storage.cc
@@ -28,25 +28,21 @@ StepOptionalAcquireExternalStorage::StepOptionalAcquireExternalStorage(
 }
 
 Step::Status StepOptionalAcquireExternalStorage::process() {
-  Storage storage = Storage::INTERNAL;
-
   PkgQueryInterface pkg_query(context_->pkgid.get(), context_->uid.get());
   std::string storage_str = pkg_query.StorageForPkgId();
-  if (!strcmp(storage_str.c_str(), kInstalledExternally))
-    storage = Storage::EXTERNAL;
-  else
-    storage = Storage::INTERNAL;
-
-  if (storage == Storage::EXTERNAL)
-    context_->external_storage =
-        ExternalStorage::AcquireExternalStorage(context_->request_type.get(),
-            context_->root_application_path.get(),
-            context_->pkgid.get(),
-            context_->pkg_type.get(),
-            context_->unpacked_dir_path.get(),
-            context_->uid.get());
-
-  if (storage == Storage::EXTERNAL && !context_->external_storage)
+
+  if (strcmp(storage_str.c_str(), kInstalledExternally))
+    return Status::OK;
+
+  context_->external_storage =
+      ExternalStorage::AcquireExternalStorage(context_->request_type.get(),
+          context_->root_application_path.get(),
+          context_->pkgid.get(),
+          context_->pkg_type.get(),
+          context_->unpacked_dir_path.get(),
+          context_->uid.get());
+
+  if (!context_->external_storage)
       LOG(WARNING) << "Cannot initialize external storage "
                    << "for uninstalled package";
 

diff --git a/src/common/step/filesystem/step_remove_files.cc b/src/common/step/filesystem/step_remove_files.cc
index a36f7015b2bbd1a11aebe82c997f5439d28f8652..12c843ed77d5011bc1e28b449e4eca2db4bea24e 100644 (file)
--- a/src/common/step/filesystem/step_remove_files.cc
+++ b/src/common/step/filesystem/step_remove_files.cc
@@ -74,8 +74,7 @@ Step::Status StepRemoveFiles::precheck() {
 
 Step::Status StepRemoveFiles::process() {
   // Use RootAppPath + Pkgid because of ReadonlyUpdateUninstall
-  bf::path pkg_path =
-      context_->root_application_path.get() / context_->pkgid.get();
+  bf::path pkg_path = context_->GetPkgPath();
 
   // We need to unmount external storage before removing package directory
   // because mount point is inside

diff --git a/src/common/step/pkgmgr/step_update_pkg_disable_info.cc b/src/common/step/pkgmgr/step_update_pkg_disable_info.cc
index ed7759988a7068f84e7dae39dce86c72890cc06f..ec7cb3bca151147d06bd14d773486e2f798af369 100644 (file)
--- a/src/common/step/pkgmgr/step_update_pkg_disable_info.cc
+++ b/src/common/step/pkgmgr/step_update_pkg_disable_info.cc
@@ -42,7 +42,7 @@ Step::Status StepUpdatePkgDisableInfo::process() {
 }
 
 Step::Status StepUpdatePkgDisableInfo::undo() {
-  if (action_type_ ==  ActionType::Disable) {
+  if (action_type_ == ActionType::Disable) {
     if (!EnablePkgInPkgmgr(context_->pkgid.get(), context_->uid.get(),
                            context_->request_mode.get())) {
       LOG(ERROR) << "Failed to update pkg info to enable : "

diff --git a/src/common/step/security/step_revoke_trust_anchor.cc b/src/common/step/security/step_revoke_trust_anchor.cc
index d95a7cde55b182451f0b0316c4b20e89a3e9dce7..bf299c7d0b1f8763e7f3397e767adeceab66f437 100644 (file)
--- a/src/common/step/security/step_revoke_trust_anchor.cc
+++ b/src/common/step/security/step_revoke_trust_anchor.cc
@@ -24,10 +24,6 @@ const char kWgt[] = "wgt";
 
 }  // namespace
 
-StepRevokeTrustAnchor::StepRevokeTrustAnchor(InstallerContext* context)
-    : Step(context) {
-}
-
 Step::Status StepRevokeTrustAnchor::undo() {
   manifest_x* manifest = context_->old_manifest_data.get();
   if (!manifest) {

diff --git a/src/common/step/security/step_revoke_trust_anchor.h b/src/common/step/security/step_revoke_trust_anchor.h
index 444545f0e14f622d7c3fd57ebe780d26c0f20e59..3ba28e7b1131fd240abf5f26fc8935e351548247 100644 (file)
--- a/src/common/step/security/step_revoke_trust_anchor.h
+++ b/src/common/step/security/step_revoke_trust_anchor.h
@@ -19,8 +19,6 @@ class StepRevokeTrustAnchor : public Step {
     UPDATE    // Update trust anchor with existing package
   };
 
-  explicit StepRevokeTrustAnchor(common_installer::InstallerContext* context);
-
   using Step::Step;
 
   Status process() override { return Status::OK; }