From: Changgyu Choi <changyu.choi@samsung.com>
Date: Fri, 28 Apr 2023 02:29:01 +0000 (+0900)
Subject: Refactor bundle implementation
X-Git-Tag: accepted/tizen/8.0/unified/20231005.093146~3
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F50%2F292150%2F6;p=platform%2Fcore%2Fbase%2Fbundle.git

Refactor bundle implementation

Change-Id: I2a5b3509df7f5769b4921d6e0253325a15d2d001
Signed-off-by: Changgyu Choi <changyu.choi@samsung.com>
---

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7171601..b22b34a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -18,7 +18,7 @@ FOREACH(flag ${pkgs_CFLAGS})
 ENDFOREACH(flag)
 SET(EXTRA_CFLAGS "${EXTRA_CFLAGS} -fvisibility=hidden -flto -Wall -Werror -Winline -Wno-noexcept-type")
 
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++11")
+SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++17")
 SET(CMAKE_CXX_FLAGS_DEBUG "-O0 -g")
 SET(CMAKE_CXX_FLAGS_RELEASE "-O2")
 
diff --git a/parcel/CMakeLists.txt b/parcel/CMakeLists.txt
index cac7578..435b6d5 100644
--- a/parcel/CMakeLists.txt
+++ b/parcel/CMakeLists.txt
@@ -9,7 +9,7 @@ FOREACH(flag ${pkgs_CFLAGS})
 ENDFOREACH(flag)
 SET(EXTRA_CFLAGS "${EXTRA_CFLAGS} -fvisibility=hidden -flto -Wall -Werror -Winline")
 
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++14")
+SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++17")
 SET(CMAKE_CXX_FLAGS_DEBUG "-O0 -g")
 SET(CMAKE_CXX_FLAGS_RELEASE "-O2")
 
diff --git a/src/bundle-internal.cc b/src/bundle-internal.cc
index 44942ac..f63dfe8 100644
--- a/src/bundle-internal.cc
+++ b/src/bundle-internal.cc
@@ -248,22 +248,19 @@ int Bundle::DecodeRaw(unsigned char* raw, size_t size) {
         &bytes[reader], &bytes[reader] + total_size);
     reader += total_size;
 
-    KeyInfo* new_key_info;
     try {
-      new_key_info = new KeyInfo(std::move(encoded_bytes));
+      auto key_info = std::make_shared<KeyInfo>(std::move(encoded_bytes));
+      auto iter = map_.find(key_info->GetKey());
+      if (iter != map_.end())
+        continue;
+
+      map_[key_info->GetKey()] = key_info;
+      list_.push_back(std::move(key_info));
     } catch (const Exception& e) {
       return e.GetErrorCode();
     } catch (const std::bad_alloc& ba) {
       return BUNDLE_ERROR_OUT_OF_MEMORY;
     }
-
-    auto key_info = std::shared_ptr<KeyInfo>(new_key_info);
-    auto iter = map_.find(key_info->GetKey());
-    if (iter != map_.end())
-      continue;
-
-    map_[key_info->GetKey()] = key_info;
-    list_.push_back(std::move(key_info));
   }
 
   return BUNDLE_ERROR_NONE;
@@ -303,23 +300,22 @@ int Bundle::Import(int argc, char** argv) {
       auto len = strlen(argv[idx + 1]) + 1;
       std::vector<unsigned char> value(p, p + len);
 
-      KeyInfo* new_key_info;
       try {
-        new_key_info = new KeyInfo(Type::String, argv[idx], std::move(value));
+        auto key_info = std::make_shared<KeyInfo>(Type::String, argv[idx],
+            std::move(value));
+        auto iter = map_.find(key_info->GetKey());
+        if (iter != map_.end())
+          continue;
+
+        map_[key_info->GetKey()] = key_info;
+        list_.push_back(std::move(key_info));
       } catch (const Exception& e) {
         return e.GetErrorCode();
       } catch (const std::bad_alloc& ba) {
         return BUNDLE_ERROR_OUT_OF_MEMORY;
       }
-
-      auto key_info = std::shared_ptr<KeyInfo>(new_key_info);
-      auto iter = map_.find(key_info->GetKey());
-      if (iter != map_.end())
-        continue;
-
-      map_[key_info->GetKey()] = key_info;
-      list_.push_back(key_info);
     }
+
     return BUNDLE_ERROR_NONE;
   }
 
@@ -337,22 +333,19 @@ int Bundle::Import(int argc, char** argv) {
     auto* p = reinterpret_cast<unsigned char*>(bytes);
     std::vector<unsigned char> decoded_bytes(p, p + (out_len + 1));
 
-    KeyInfo* new_key_info;
     try {
-      new_key_info = new KeyInfo(std::move(decoded_bytes));
+      auto key_info = std::make_shared<KeyInfo>(std::move(decoded_bytes));
+      auto iter = map_.find(key_info->GetKey());
+      if (iter != map_.end())
+        continue;
+
+      map_[key_info->GetKey()] = key_info;
+      list_.push_back(std::move(key_info));
     } catch (const Exception& e) {
       return e.GetErrorCode();
     } catch (const std::bad_alloc& ba) {
       return BUNDLE_ERROR_OUT_OF_MEMORY;
     }
-
-    auto key_info = std::shared_ptr<KeyInfo>(new_key_info);
-    auto iter = map_.find(key_info->GetKey());
-    if (iter != map_.end())
-      continue;
-
-    map_[key_info->GetKey()] = key_info;
-    list_.push_back(std::move(key_info));
   }
 
   return BUNDLE_ERROR_NONE;
diff --git a/src/key-info-internal.cc b/src/key-info-internal.cc
index e7c7f6f..e5d0b03 100644
--- a/src/key-info-internal.cc
+++ b/src/key-info-internal.cc
@@ -14,7 +14,9 @@
  * limitations under the License.
  */
 
+#include <algorithm>
 #include <cstring>
+#include <utility>
 
 #include "include/bundle.h"
 
@@ -24,10 +26,8 @@
 namespace tizen_base {
 namespace internal {
 
-KeyInfo::KeyInfo(int type, std::string key,
-    std::vector<unsigned char> value)
-  : type_(type),
-    key_(std::move(key)) {
+KeyInfo::KeyInfo(int type, std::string key, std::vector<unsigned char> value)
+    : type_(type), key_(std::move(key)) {
   int ret = SetValue(value);
   if (ret != BUNDLE_ERROR_NONE)
     THROW(ret);
@@ -35,8 +35,7 @@ KeyInfo::KeyInfo(int type, std::string key,
 
 KeyInfo::KeyInfo(int type, std::string key,
     std::vector<std::vector<unsigned char>> values)
-  : type_(type),
-    key_(std::move(key)) {
+    : type_(type), key_(std::move(key)) {
   int ret = SetValues(values);
   if (ret != BUNDLE_ERROR_NONE)
     THROW(ret);
@@ -49,37 +48,49 @@ KeyInfo::KeyInfo(std::vector<unsigned char> encoded_bytes) {
 }
 
 KeyInfo::KeyInfo(const KeyInfo& key_info) {
+  values_.reserve(key_info.values_.size());
+  try {
+    for (unsigned int i = 0; i < key_info.values_.size(); ++i) {
+      auto new_value =
+          std::make_unique<unsigned char[]>(key_info.values_size_[i]);
+      std::copy(key_info.values_[i].get(),
+          key_info.values_[i].get() + key_info.values_size_[i],
+          new_value.get());
+      values_.push_back(std::move(new_value));
+    }
+  } catch (const std::bad_alloc& e) {
+    THROW(BUNDLE_ERROR_OUT_OF_MEMORY);
+  }
+
   type_ = key_info.type_;
   key_ = key_info.key_;
   values_size_ = key_info.values_size_;
   uvalues_size_ = key_info.uvalues_size_;
-  for (unsigned int i = 0; i < key_info.values_.size(); ++i) {
-    auto* new_value = new (std::nothrow) unsigned char[values_size_[i]];
-    if (new_value == nullptr)
-      THROW(BUNDLE_ERROR_OUT_OF_MEMORY);
-
-    std::copy(key_info.values_[i].get(),
-        key_info.values_[i].get() + values_size_[i], new_value);
-    values_.emplace_back(new_value);
-  }
 }
 
-KeyInfo& KeyInfo::operator = (const KeyInfo& key_info) {
+KeyInfo& KeyInfo::operator=(const KeyInfo& key_info) {
   if (this != &key_info) {
+    decltype(values_) new_values;
+    try {
+      for (unsigned int i = 0; i < key_info.values_.size(); ++i) {
+        auto new_value =
+            std::make_unique<unsigned char[]>(key_info.values_size_[i]);
+        std::copy(key_info.values_[i].get(),
+            key_info.values_[i].get() + key_info.values_size_[i],
+            new_value.get());
+        new_values.push_back(std::move(new_value));
+      }
+    } catch (const std::bad_alloc& e) {
+      THROW(BUNDLE_ERROR_OUT_OF_MEMORY);
+    }
+
     type_ = key_info.type_;
     key_ = key_info.key_;
     values_size_ = key_info.values_size_;
     uvalues_size_ = key_info.uvalues_size_;
-    for (unsigned int i = 0; i < key_info.values_.size(); ++i) {
-      auto* new_value = new (std::nothrow) unsigned char[values_size_[i]];
-      if (new_value == nullptr)
-        THROW(BUNDLE_ERROR_OUT_OF_MEMORY);
-
-      std::copy(key_info.values_[i].get(),
-          key_info.values_[i].get() + values_size_[i], new_value);
-      values_.emplace_back(new_value);
-    }
+    values_ = std::move(new_values);
   }
+
   return *this;
 }
 
@@ -92,7 +103,7 @@ KeyInfo::KeyInfo(KeyInfo&& key_info) noexcept {
   uvalues_size_ = std::move(key_info.uvalues_size_);
 }
 
-KeyInfo& KeyInfo::operator = (KeyInfo&& key_info) noexcept {
+KeyInfo& KeyInfo::operator=(KeyInfo&& key_info) noexcept {
   if (this != &key_info) {
     type_ = key_info.type_;
     key_info.type_ = 0;
@@ -104,7 +115,7 @@ KeyInfo& KeyInfo::operator = (KeyInfo&& key_info) noexcept {
   return *this;
 }
 
-bool KeyInfo::operator == (const KeyInfo& key_info) {
+bool KeyInfo::operator==(const KeyInfo& key_info) {
   if (this == &key_info)
     return true;
 
@@ -118,12 +129,12 @@ bool KeyInfo::operator == (const KeyInfo& key_info) {
     return false;
 
   for (unsigned int i = 0; i < values_.size(); ++i) {
-    if (values_size_[i] == key_info.values_size_[i])
+    if (values_size_[i] != key_info.values_size_[i])
       return false;
 
     int ret = std::memcmp(values_[i].get(), key_info.values_[i].get(),
         values_size_[i]);
-    if (ret == 0)
+    if (ret != 0)
       return false;
   }
 
@@ -158,28 +169,32 @@ const std::vector<unsigned int>& KeyInfo::GetUValuesSize() {
 }
 
 int KeyInfo::SetValue(const std::vector<unsigned char>& value) {
-  auto* new_value = new (std::nothrow) unsigned char[value.size()];
-  if (new_value == nullptr)
+  try {
+    auto new_value = std::make_unique<unsigned char[]>(value.size());
+    std::copy(value.begin(), value.end(), new_value.get());
+    values_.emplace_back(std::move(new_value));
+  } catch (const std::bad_alloc& e) {
     return BUNDLE_ERROR_OUT_OF_MEMORY;
+  }
 
-  std::copy(value.begin(), value.end(), new_value);
-  values_.emplace_back(new_value);
   values_size_.push_back(value.size());
   uvalues_size_.push_back(static_cast<unsigned int>(value.size()));
   return BUNDLE_ERROR_NONE;
 }
 
 int KeyInfo::SetValues(const std::vector<std::vector<unsigned char>>& values) {
-  for (unsigned int i = 0; i< values.size(); ++i) {
-    auto* new_value = new (std::nothrow) unsigned char[values[i].size()];
-    if (new_value == nullptr)
-      return BUNDLE_ERROR_OUT_OF_MEMORY;
-
-    std::copy(values[i].begin(), values[i].end(), new_value);
-    values_.emplace_back(new_value);
-    values_size_.push_back(values[i].size());
-    uvalues_size_.push_back(values[i].size());
+  try {
+    for (unsigned int i = 0; i < values.size(); ++i) {
+      auto new_value = std::make_unique<unsigned char[]>(values[i].size());
+      std::copy(values[i].begin(), values[i].end(), new_value.get());
+      values_.push_back(std::move(new_value));
+      values_size_.push_back(values[i].size());
+      uvalues_size_.push_back(values[i].size());
+    }
+  } catch (const std::bad_alloc& e) {
+    return BUNDLE_ERROR_OUT_OF_MEMORY;
   }
+
   return BUNDLE_ERROR_NONE;
 }
 
@@ -210,7 +225,7 @@ std::vector<unsigned char> KeyInfo::Encode() {
     // values size
     std::size_t values_size = values_.size();
     p = reinterpret_cast<unsigned char*>(&values_size);
-    bytes.insert(bytes.end(), p , p + sizeof(values_size));
+    bytes.insert(bytes.end(), p, p + sizeof(values_size));
   }
 
   // values
@@ -290,7 +305,7 @@ int KeyInfo::Decode(const std::vector<unsigned char>& bytes) {
     return BUNDLE_ERROR_INVALID_PARAMETER;
 
   p = reinterpret_cast<unsigned char*>(&type_);
-  std::copy(&bytes[reader], &bytes[reader] + sizeof(type_),  p);
+  std::copy(&bytes[reader], &bytes[reader] + sizeof(type_), p);
   reader += sizeof(type_);
 
   // key size
@@ -340,14 +355,14 @@ int KeyInfo::Decode(const std::vector<unsigned char>& bytes) {
     if ((reader + value_size) > bytes.size())
       return BUNDLE_ERROR_INVALID_PARAMETER;
 
-    auto* new_value = new (std::nothrow) unsigned char[value_size];
-    if (new_value == nullptr)
+    try {
+      auto new_value = std::make_unique<unsigned char[]>(value_size);
+      std::copy(&bytes[reader], &bytes[reader] + value_size, new_value.get());
+      reader += value_size;
+      values_.push_back(std::move(new_value));
+    } catch (const std::bad_alloc& e) {
       return BUNDLE_ERROR_OUT_OF_MEMORY;
-
-    std::copy(&bytes[reader], &bytes[reader] + value_size, new_value);
-    reader += value_size;
-
-    values_.emplace_back(new_value);
+    }
     values_size_.push_back(value_size);
     uvalues_size_.push_back(static_cast<unsigned int>(value_size));
   }
@@ -362,14 +377,16 @@ int KeyInfo::SetValue(int index, const std::vector<unsigned char>& value) {
   if (!IsArray())
     return BUNDLE_ERROR_INVALID_PARAMETER;
 
-  auto* new_value = new (std::nothrow) unsigned char[value.size()];
-  if (new_value == nullptr)
-    return BUNDLE_ERROR_OUT_OF_MEMORY;
+  try {
+    auto new_value = std::make_unique<unsigned char[]>(value.size());
+    if (value.size() != 0)
+      std::copy(value.begin(), value.end(), new_value.get());
 
-  if (value.size() != 0)
-    std::copy(value.begin(), value.end(), new_value);
+    values_[index] = std::move(new_value);
+  } catch (const std::bad_alloc& e) {
+    return BUNDLE_ERROR_OUT_OF_MEMORY;
+  }
 
-  values_[index].reset(new_value);
   values_size_[index] = value.size();
   uvalues_size_[index] = static_cast<unsigned int>(value.size());
   return BUNDLE_ERROR_NONE;
diff --git a/src/stub.cc b/src/stub.cc
index fb4e933..9f555b6 100644
--- a/src/stub.cc
+++ b/src/stub.cc
@@ -143,8 +143,7 @@ extern "C" EXPORT_API void bundle_iterate(bundle* b,
   }
 
   auto* h = reinterpret_cast<Bundle*>(b);
-  for (const auto& kv : h->GetMap()) {
-    auto& key_info = kv.second;
+  for (const auto& [key, key_info] : h->GetMap()) {
     auto& values = key_info->GetValues();
     auto& value = const_cast<std::unique_ptr<unsigned char[]>&>(values[0]);
     auto* val = reinterpret_cast<char*>(&value[0]);
@@ -429,18 +428,14 @@ extern "C" EXPORT_API int bundle_add_str_array(bundle* b, const char* key,
     }
   }
 
-  KeyInfo* key_info;
   try {
-    key_info = new KeyInfo((BUNDLE_TYPE_STR_ARRAY | BUNDLE_TYPE_ARRAY),
-        key, values);
-  } catch (const Exception& e) {
-    return e.GetErrorCode();
-  }
-
-  try {
-    h->Add(std::shared_ptr<KeyInfo>(key_info));
+    auto key_info = std::make_shared<KeyInfo>(
+        (BUNDLE_TYPE_STR_ARRAY | BUNDLE_TYPE_ARRAY), key, values);
+    h->Add(std::move(key_info));
   } catch (const Exception& e) {
     return e.GetErrorCode();
+  } catch (const std::bad_alloc& ba) {
+    return BUNDLE_ERROR_OUT_OF_MEMORY;
   }
 
   return BUNDLE_ERROR_NONE;
diff --git a/tests/bundle_unittests/CMakeLists.txt b/tests/bundle_unittests/CMakeLists.txt
index a7eb6d5..7ee51e9 100644
--- a/tests/bundle_unittests/CMakeLists.txt
+++ b/tests/bundle_unittests/CMakeLists.txt
@@ -15,7 +15,7 @@ FOREACH(flag ${bundle_unittests_CFLAGS})
 ENDFOREACH(flag)
 SET(EXTRA_CFLAGS "${EXTRA_CFLAGS} -fvisibility=hidden -Wall -Werror -Winline")
 
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++11")
+SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++17")
 SET(CMAKE_CXX_FLAGS_DEBUG "-O0 -g")
 SET(CMAKE_CXX_FLAGS_RELEASE "-O2")
 
diff --git a/tests/parcel_unittests/CMakeLists.txt b/tests/parcel_unittests/CMakeLists.txt
index d481e02..b2df484 100644
--- a/tests/parcel_unittests/CMakeLists.txt
+++ b/tests/parcel_unittests/CMakeLists.txt
@@ -15,7 +15,7 @@ FOREACH(flag ${parcel_unittests_CFLAGS})
 ENDFOREACH(flag)
 SET(EXTRA_CFLAGS "${EXTRA_CFLAGS} -fvisibility=hidden -Wall -Werror -Winline")
 
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++14")
+SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -std=c++17")
 SET(CMAKE_CXX_FLAGS_DEBUG "-O0 -g")
 SET(CMAKE_CXX_FLAGS_RELEASE "-O2")