From 67b70bf18df254ef7b1e4f62349d035d96413663 Mon Sep 17 00:00:00 2001
From: Junghyun Yeon <jungh.yeon@samsung.com>
Date: Tue, 5 Nov 2019 20:52:45 +0900
Subject: [PATCH] Fix codes for efficiency

- Remove unnecessary privilege checking routine.
- Integrate if statements to remove redundant exit checking routine.

Change-Id: I575e46ae134f1799cd4546491e17f82121c91795
Signed-off-by: Junghyun Yeon <jungh.yeon@samsung.com>
---
 src/common/shared_dirs.cc                          | 34 ++++++++--------------
 .../filesystem/step_create_storage_directories.cc  | 10 ++++---
 .../filesystem/step_update_storage_directories.cc  | 13 +++++----
 3 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/src/common/shared_dirs.cc b/src/common/shared_dirs.cc
index da66546..c551f57 100644
--- a/src/common/shared_dirs.cc
+++ b/src/common/shared_dirs.cc
@@ -77,30 +77,21 @@ const char kSystemShareGroupName[] = "system_share";
 
 bool SetDirectoryOwnerAndPermissions(const bf::path& subpath, uid_t uid,
                                      gid_t gid) {
-  bf::perms perms = bf::owner_read |
-                    bf::owner_write |
-                    bf::group_read;
-  // symlink will be skipped
-  if (bf::is_symlink(symlink_status(subpath)))
+  // symlink and non directory will be skipped
+  if (bf::is_symlink(symlink_status(subpath)) || !bf::is_directory(subpath))
     return true;
 
-  // non directory will be skipped
-  bool result = true;
-  if (bf::is_directory(subpath)) {
-    perms |= bf::owner_exe | bf::group_exe | bf::others_read | bf::others_exe;
-    if (subpath.filename() == "data" || subpath.filename() == "cache") {
-      perms |= bf::group_write | bf::set_gid_on_exe;
-      boost::optional<gid_t> system_share =
-          ci::GetGidByGroupName(kSystemShareGroupName);
-      if (!system_share)
-        return false;
-      gid = *system_share;
-    }
-    result = common_installer::SetDirOwnershipAndPermissions(subpath, perms,
-                                                             uid, gid);
+  bf::perms perms = bf::all_all ^ bf::group_write ^ bf::others_write;
+  if (subpath.filename() == "data" || subpath.filename() == "cache") {
+    perms |= bf::group_write | bf::set_gid_on_exe;
+    boost::optional<gid_t> system_share =
+        ci::GetGidByGroupName(kSystemShareGroupName);
+    if (!system_share)
+      return false;
+    gid = *system_share;
   }
-
-  return result;
+  return common_installer::SetDirOwnershipAndPermissions(subpath, perms,
+                                                         uid, gid);
 }
 
 bool CreateDirectories(const bf::path& app_dir, const std::string& pkgid,
@@ -577,7 +568,6 @@ bool DeleteUserExternalDirectories(const std::string& pkgid) {
   return true;
 }
 
-
 bool CopyUserDirectories(const std::string& pkgid) {
   UserList list = ci::GetUserList();
   for (auto l : list) {
diff --git a/src/common/step/filesystem/step_create_storage_directories.cc b/src/common/step/filesystem/step_create_storage_directories.cc
index de305e5..fa359d8 100644
--- a/src/common/step/filesystem/step_create_storage_directories.cc
+++ b/src/common/step/filesystem/step_create_storage_directories.cc
@@ -74,10 +74,12 @@ Step::Status StepCreateStorageDirectories::process() {
 
   manifest_x* manifest = context_->manifest_data.get();
   bool shareddata = ShouldSupportLegacySharedDataDir(manifest->api_version);
-  for (auto& priv : GListRange<privilege_x*>(manifest->privileges)) {
-    if (!strcmp(priv->value, privileges::kPrivForSharedData)) {
-      shareddata = true;
-      break;
+  if (!shareddata) {
+    for (auto& priv : GListRange<privilege_x*>(manifest->privileges)) {
+      if (!strcmp(priv->value, privileges::kPrivForSharedData)) {
+        shareddata = true;
+        break;
+      }
     }
   }
 
diff --git a/src/common/step/filesystem/step_update_storage_directories.cc b/src/common/step/filesystem/step_update_storage_directories.cc
index 3ebc00a..657a602 100644
--- a/src/common/step/filesystem/step_update_storage_directories.cc
+++ b/src/common/step/filesystem/step_update_storage_directories.cc
@@ -69,14 +69,15 @@ bool CreatePerUserSharedDir(const std::string& pkgid) {
 }
 
 bool ShouldCreateSharedDataDir(manifest_x* manifest) {
-  bool shareddata = ci::ShouldSupportLegacySharedDataDir(manifest->api_version);
+  if (ci::ShouldSupportLegacySharedDataDir(manifest->api_version))
+    return true;
+
   for (auto& priv : GListRange<privilege_x*>(manifest->privileges)) {
-    if (!strcmp(priv->value, ci::privileges::kPrivForSharedData)) {
-      shareddata = true;
-      break;
-    }
+    if (!strcmp(priv->value, ci::privileges::kPrivForSharedData))
+      return true;
   }
-  return shareddata;
+
+  return false;
 }
 
 }  // namespace
-- 
2.7.4