Refactor and reduce unnecessary code

Change-Id: I09bedb005965173688050ce7d46471928ce2084c
Signed-off-by: Damian Pietruchowski <d.pietruchow@samsung.com>

diff --git a/src/common/certificate_validation.cc b/src/common/certificate_validation.cc
index 3526af425edf77613658a6b64487f8685a9dd1e8..9c2fa40e31490189ad0790a18077ad2cde48d4ef 100644 (file)
--- a/src/common/certificate_validation.cc
+++ b/src/common/certificate_validation.cc
@@ -26,36 +26,34 @@ namespace {
 const char kSignatureAuthor[] = "author-signature.xml";
 const char kRegexDistributorSignature[] = "^(signature)([1-9][0-9]*)(\\.xml)";
 
-
-bool SetAuthorCertificate(ValidationCore::SignatureData data,
-    common_installer::CertificateInfo* cert_info) {
+static bool SetCertificate(const ValidationCore::SignatureData& data,
+    Property<ValidationCore::CertificatePtr>* certificate,
+    Property<ValidationCore::CertificatePtr>* intermediate_certificate,
+    Property<ValidationCore::CertificatePtr>* root_certificate,
+    bool intermediate_mandatory) {
   ValidationCore::CertificateList cert_list = data.getCertList();
   ValidationCore::CertificateList::iterator it = cert_list.begin();
   if (it == cert_list.end()) {
     LOG(ERROR) << "No certificates in certificate list";
     return false;
   }
-  unsigned char* public_key;
-  size_t len;
-  (*it)->getPublicKeyDER(&public_key, &len);
-  std::string author_id =
-      ci::EncodeBase64(public_key, len);
-  cert_info->author_id.set(author_id);
-  cert_info->author_certificate.set(*it);
-  // cert_list has at least 3 certificates: end-user, intermediate, root
-  // currently pkgmgr can store only one intermediate cert, so just set
-  // first intermediate cert here.
+  certificate->set(*it);
   ++it;
   if (it == cert_list.end()) {
-    LOG(ERROR) << "No intermediate certificates in certificate list";
-    return false;
+    if (intermediate_mandatory) {
+      LOG(ERROR) << "No intermediate certificates in certificate list";
+      return false;
+    } else {
+      intermediate_certificate->set({});
+    }
+  } else {
+    intermediate_certificate->set(*it);
   }
-  cert_info->author_intermediate_certificate.set(*it);
-  cert_info->author_root_certificate.set(data.getRootCaCertificatePtr());
+  root_certificate->set(data.getRootCaCertificatePtr());
   return true;
 }
 
-bool SetDistributorCertificate(ValidationCore::SignatureData data,
+static bool SetAuthorCertificate(const ValidationCore::SignatureData& data,
     common_installer::CertificateInfo* cert_info) {
   ValidationCore::CertificateList cert_list = data.getCertList();
   ValidationCore::CertificateList::iterator it = cert_list.begin();
@@ -63,35 +61,35 @@ bool SetDistributorCertificate(ValidationCore::SignatureData data,
     LOG(ERROR) << "No certificates in certificate list";
     return false;
   }
-  cert_info->distributor_certificate.set(*it);
-  ++it;
-  if (it == cert_list.end()) {
-    LOG(ERROR) << "No intermediate certificates in certificate list";
-    return false;
-  }
-  cert_info->distributor_intermediate_certificate.set(*it);
-  cert_info->distributor_root_certificate.set(data.getRootCaCertificatePtr());
-  return true;
+  unsigned char* public_key;
+  size_t len;
+  (*it)->getPublicKeyDER(&public_key, &len);
+  std::string author_id =
+      ci::EncodeBase64(public_key, len);
+  cert_info->author_id.set(author_id);
+  return SetCertificate(data,
+      &cert_info->author_certificate,
+      &cert_info->author_intermediate_certificate,
+      &cert_info->author_root_certificate,
+      true);
 }
 
-bool SetDistributor2Certificate(ValidationCore::SignatureData data,
+bool SetDistributorCertificate(const ValidationCore::SignatureData& data,
     common_installer::CertificateInfo* cert_info) {
-  ValidationCore::CertificateList cert_list = data.getCertList();
-  ValidationCore::CertificateList::iterator it = cert_list.begin();
-  if (it == cert_list.end()) {
-    LOG(ERROR) << "No certificates in certificate list";
-    return false;
-  }
-  cert_info->distributor2_certificate.set(*it);
-  ++it;
-  if (it == cert_list.end()) {
-    LOG(INFO) << "No intermediate certificates in certificate list";
-    cert_info->distributor2_intermediate_certificate.set({});
-  } else {
-    cert_info->distributor2_intermediate_certificate.set(*it);
-  }
-  cert_info->distributor2_root_certificate.set(data.getRootCaCertificatePtr());
-  return true;
+  return SetCertificate(data,
+      &cert_info->distributor_certificate,
+      &cert_info->distributor_intermediate_certificate,
+      &cert_info->distributor_root_certificate,
+      true);
+}
+
+bool SetDistributor2Certificate(const ValidationCore::SignatureData& data,
+    common_installer::CertificateInfo* cert_info) {
+  return SetCertificate(data,
+      &cert_info->distributor2_certificate,
+      &cert_info->distributor2_intermediate_certificate,
+      &cert_info->distributor2_root_certificate,
+      false);
 }
 
 }  // namespace
@@ -126,7 +124,7 @@ privilege_manager_visibility_e PrivilegeLevelToVisibility(
   }
 }
 
-void SetPrivilegeLevel(ValidationCore::SignatureData data,
+void SetPrivilegeLevel(const ValidationCore::SignatureData& data,
     common_installer::PrivilegeLevel* level) {
   // already set
   if (*level != common_installer::PrivilegeLevel::UNTRUSTED)

diff --git a/src/common/certificate_validation.h b/src/common/certificate_validation.h
index de5db7f9b4b1ba0054351386527a516cea8b2b30..57f8338bc0c4a6a4fa8bfa708de60e9f9da80e1f 100644 (file)
--- a/src/common/certificate_validation.h
+++ b/src/common/certificate_validation.h
@@ -23,7 +23,7 @@ common_installer::PrivilegeLevel CertStoreIdToPrivilegeLevel(
 privilege_manager_visibility_e PrivilegeLevelToVisibility(
     common_installer::PrivilegeLevel level);
 
-void SetPrivilegeLevel(ValidationCore::SignatureData data,
+void SetPrivilegeLevel(const ValidationCore::SignatureData& data,
     common_installer::PrivilegeLevel* level);
 
 bool ValidateSignatureFile(

diff --git a/src/common/shared_dirs.cc b/src/common/shared_dirs.cc
index f8d6343aa651ac29c7693b4be1bbaf8d438a08c0..8b00472ef5e3c6d22111cd1593b754d1733b9b44 100644 (file)
--- a/src/common/shared_dirs.cc
+++ b/src/common/shared_dirs.cc
@@ -22,7 +22,6 @@
 #include <tzplatform_config.h>
 #include <sys/xattr.h>
 
-
 #include <algorithm>
 #include <cassert>
 #include <cstring>
@@ -74,40 +73,8 @@ const char kExternalStoragePrivilege[] =
     "http://tizen.org/privilege/externalstorage.appdata";
 const char kSystemShareGroupName[] = "system_share";
 
-bool SetFileOwner(const bf::path& subpath, uid_t uid, gid_t gid) {
-  bs::error_code error;
-  int fd = open(subpath.c_str(), O_RDONLY);
-  if (fd < 0) {
-    LOG(ERROR) << "Can't open directory : " << subpath;
-    return false;
-  }
-  int ret = fchown(fd, uid, gid);
-  close(fd);
-  if (ret != 0) {
-    LOG(ERROR) << "Failed to change owner of: " << subpath;
-    return false;
-  }
-  return true;
-}
-
-bool SetOwnerAndPermissions(const bf::path& subpath, uid_t uid,
-                            gid_t gid, bf::perms perms) {
-  bs::error_code error;
-  bf::permissions(subpath, perms, error);
-  if (error) {
-    LOG(ERROR) << "Failed to set permissions for: " << subpath;
-    return false;
-  }
-
-  if (!SetFileOwner(subpath, uid, gid)) {
-    return false;
-  }
-  return true;
-}
-
 bool SetDirectoryOwnerAndPermissions(const bf::path& subpath, uid_t uid,
-                                            gid_t gid) {
-  bs::error_code error;
+                                     gid_t gid) {
   bf::perms perms = bf::owner_read |
                     bf::owner_write |
                     bf::group_read;
@@ -127,7 +94,8 @@ bool SetDirectoryOwnerAndPermissions(const bf::path& subpath, uid_t uid,
         return false;
       gid = *system_share;
     }
-    result = SetOwnerAndPermissions(subpath, uid, gid, perms);
+    result = common_installer::SetDirOwnershipAndPermissions(subpath, perms,
+                                                             uid, gid);
   }
 
   return result;
@@ -786,11 +754,11 @@ bool SetPackageDirectoryOwnerAndPermissions(const bf::path& path, uid_t uid) {
     } else if (iter.level() == 1 &&
         iter->path().parent_path().filename() == "bin") {
       // bin files
-      if (!SetOwnerAndPermissions(iter->path(), uid, *gid, perms755))
+      if (!SetDirOwnershipAndPermissions(iter->path(), perms755, uid, *gid))
         return false;
     } else {
       // other files
-      if (!SetOwnerAndPermissions(iter->path(), uid, *gid, perms644))
+      if (!SetDirOwnershipAndPermissions(iter->path(), perms644, uid, *gid))
         return false;
     }
   }

diff --git a/src/common/step/filesystem/step_move_installed_storage.cc b/src/common/step/filesystem/step_move_installed_storage.cc
index bf9a05817889499068f53ead5a5656dad40ccfa5..db1bce11dfd3587102add62dae8da59813b9b879 100644 (file)
--- a/src/common/step/filesystem/step_move_installed_storage.cc
+++ b/src/common/step/filesystem/step_move_installed_storage.cc
@@ -25,6 +25,26 @@ namespace {
 
 const char kInternalOnly[] = "internal-only";
 
+bool MoveFileAndUpdateTepInfo(const boost::filesystem::path& src,
+    const boost::filesystem::path& dst,
+    const common_installer::InstallerContext* context) {
+  if (!bf::exists(src))
+    return true;
+
+  if (!common_installer::MoveFile(src, dst)) {
+    LOG(ERROR) << "Cannot move tep file from: " << src
+               << " to " << dst;
+    return false;
+  }
+
+  if (!common_installer::UpdateTepInfoInPkgmgr(dst, context->pkgid.get(),
+      context->uid.get(), context->request_mode.get())) {
+    LOG(ERROR) << "Failed to update tep package location in pkgmgr";
+    return false;
+  }
+  return true;
+}
+
 }  // namespace
 
 namespace common_installer {
@@ -106,35 +126,13 @@ bool StepMoveInstalledStorage::MoveTep() {
     }
   }
 
-  if (!MoveFile(old_tep_location_, new_tep_location_)) {
-    LOG(ERROR) << "Cannot move tep file from: " << old_tep_location_
-               << " to " << new_tep_location_;
-    return false;
-  }
-
-  if (!UpdateTepInfoInPkgmgr(new_tep_location_, context_->pkgid.get(),
-      context_->uid.get(), context_->request_mode.get())) {
-    LOG(ERROR) << "Failed to update tep package location in pkgmgr";
-    return false;
-  }
-  return true;
+  return MoveFileAndUpdateTepInfo(old_tep_location_, new_tep_location_,
+      context_);
 }
 
 bool StepMoveInstalledStorage::MoveBackTep() {
-  if (bf::exists(new_tep_location_)) {
-    if (!MoveFile(new_tep_location_, old_tep_location_)) {
-      LOG(ERROR) << "Cannot move tep file from: " << new_tep_location_
-                 << " to " << old_tep_location_;
-      return false;
-    }
-
-    if (!UpdateTepInfoInPkgmgr(old_tep_location_, context_->pkgid.get(),
-        context_->uid.get(), context_->request_mode.get())) {
-      LOG(ERROR) << "Failed to update tep package location in pkgmgr";
-      return false;
-    }
-  }
-  return true;
+  return MoveFileAndUpdateTepInfo(new_tep_location_, old_tep_location_,
+      context_);
 }
 
 bool StepMoveInstalledStorage::MoveExternal() {