From: Junghyun Yeon Date: Thu, 5 Aug 2021 02:56:06 +0000 (+0900) Subject: Improve code readability. X-Git-Tag: submit/tizen/20210906.013143~2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5400a110476cea08f95aebaec513fbe2206bfdb6;p=platform%2Fcore%2Fappfw%2Fpkgmgr-tool.git Improve code readability. - Add some empty lines for readability. - Reduce indentation depth. - Extract codes into function. Change-Id: I7e3b3199632f71632b9e7311aaf35ea7ed4037a2 Signed-off-by: Junghyun Yeon --- diff --git a/src/pkg_upgrade/src/backend_invoker.cc b/src/pkg_upgrade/src/backend_invoker.cc index 1308bad..f3a3e14 100644 --- 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 --- 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 --- 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(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::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 --- 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> PkgUpgraderFactory::MakeList(PkgFinder* finder) { @@ -47,27 +76,29 @@ list> 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( std::make_unique(*old_pkg, PkgOperation::COMPLEX), std::make_unique(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( std::make_unique(*old_pkg, PkgOperation::COMPLEX), std::make_unique(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( - std::make_unique(nullptr, - std::make_unique(*old_pkg, - PkgOperation::UNINSTALL_KEEP_RW_DATA)), - std::make_unique(nullptr, - std::make_unique(new_pkg, - PkgOperation::INSTALL)))); - } else if (old_pkg->IsReadOnly() && !new_pkg.IsReadOnly()) { - // RO to RW + std::make_unique(nullptr, + std::make_unique(*old_pkg, + PkgOperation::UNINSTALL_KEEP_RW_DATA)), + std::make_unique(nullptr, + std::make_unique(new_pkg, + PkgOperation::INSTALL)))); + break; + case UpdateType::kRo2Rw: pkgs.emplace_back(std::make_unique( std::make_unique(nullptr, std::make_unique(*old_pkg, @@ -75,6 +106,9 @@ list> PkgUpgraderFactory::Merge( std::make_unique(nullptr, std::make_unique(new_pkg, PkgOperation::INSTALL)))); + break; + default: + break; } } else { // INSTALL