From 3f7f9896801661473564730c4b196968f2fbd918 Mon Sep 17 00:00:00 2001 From: Niels Sascha Reedijk Date: Sat, 8 Aug 2020 14:44:16 +0000 Subject: [PATCH] Installer: Remove `level` parameter from CopyEngine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The level parameter in the CopyEngine::CollectCopyInfo() and CopyEngine::Copy() methods was introduced in hrev30395 to allow the CopyEngine to decide which directories should be copied. Since then, this class has been rewritten and it is no longer necessary for that purpose. This change refactors the CopyEngine and removes the level parameter from the class interface. Furthermore, it was broken to begin with; it was passed as reference to the internal recursive _Copy() and _CollectCopyInfo() methods, meaning they acted like a global counter. The global counter was increased at the beginning and decreased at the end of those methods. Execution could terminate early though, leaving the level counter out of sync with the recursion level. There is one use of the level parameter, namely in the WorkerThread::EntryFilter::ShouldClobberFolder() method, but the use of the parameter was wrong (it would have been at level 3 at the point of the check, not level 2) and the logic is functional without the level check. Change-Id: Id92ef89b015e9b1185bde061273f61e492664bce Reviewed-on: https://review.haiku-os.org/c/haiku/+/3139 Reviewed-by: Adrien Destugues Reviewed-by: Axel Dörfler --- src/apps/installer/CopyEngine.cpp | 31 ++++++++++------------------- src/apps/installer/CopyEngine.h | 15 ++++++-------- src/apps/installer/WorkerThread.cpp | 7 +++---- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/src/apps/installer/CopyEngine.cpp b/src/apps/installer/CopyEngine.cpp index 225bd90e8f..e2dab52d49 100644 --- a/src/apps/installer/CopyEngine.cpp +++ b/src/apps/installer/CopyEngine.cpp @@ -113,10 +113,9 @@ CopyEngine::ResetTargets(const char* source) status_t CopyEngine::CollectTargets(const char* source, sem_id cancelSemaphore) { - int32 level = 0; off_t bytesToCopy = 0; uint64 itemsToCopy = 0; - status_t ret = _CollectCopyInfo(source, level, cancelSemaphore, bytesToCopy, + status_t ret = _CollectCopyInfo(source, cancelSemaphore, bytesToCopy, itemsToCopy); if (ret == B_OK && fProgressReporter != NULL) fProgressReporter->AddItems(itemsToCopy, bytesToCopy); @@ -128,7 +127,6 @@ status_t CopyEngine::Copy(const char* _source, const char* _destination, sem_id cancelSemaphore, bool copyAttributes) { - int32 level = 0; status_t ret; BEntry source(_source); @@ -141,7 +139,7 @@ CopyEngine::Copy(const char* _source, const char* _destination, if (ret != B_OK) return ret; - return _Copy(source, destination, level, cancelSemaphore, copyAttributes); + return _Copy(source, destination, cancelSemaphore, copyAttributes); } @@ -230,11 +228,9 @@ CopyEngine::_CopyData(const BEntry& _source, const BEntry& _destination, status_t -CopyEngine::_CollectCopyInfo(const char* _source, int32& level, - sem_id cancelSemaphore, off_t& bytesToCopy, uint64& itemsToCopy) +CopyEngine::_CollectCopyInfo(const char* _source, sem_id cancelSemaphore, + off_t& bytesToCopy, uint64& itemsToCopy) { - level++; - BEntry source(_source); status_t ret = source.InitCheck(); if (ret < B_OK) @@ -253,7 +249,7 @@ CopyEngine::_CollectCopyInfo(const char* _source, int32& level, if (fEntryFilter != NULL && !fEntryFilter->ShouldCopyEntry(source, - _RelativeEntryPath(_source), statInfo, level)) { + _RelativeEntryPath(_source), statInfo)) { // Skip this entry return B_OK; } @@ -274,8 +270,8 @@ CopyEngine::_CollectCopyInfo(const char* _source, int32& level, if (ret < B_OK) return ret; - ret = _CollectCopyInfo(entryPath.Path(), level, - cancelSemaphore, bytesToCopy, itemsToCopy); + ret = _CollectCopyInfo(entryPath.Path(), cancelSemaphore, + bytesToCopy, itemsToCopy); if (ret < B_OK) return ret; } @@ -286,17 +282,14 @@ CopyEngine::_CollectCopyInfo(const char* _source, int32& level, } itemsToCopy++; - level--; return B_OK; } status_t CopyEngine::_Copy(BEntry &source, BEntry &destination, - int32& level, sem_id cancelSemaphore, bool copyAttributes) + sem_id cancelSemaphore, bool copyAttributes) { - level++; - struct stat sourceInfo; status_t ret = source.GetStat(&sourceInfo); if (ret != B_OK) @@ -321,7 +314,7 @@ CopyEngine::_Copy(BEntry &source, BEntry &destination, const char *relativeSourcePath = _RelativeEntryPath(sourcePath.Path()); if (fEntryFilter != NULL && !fEntryFilter->ShouldCopyEntry(source, relativeSourcePath, - sourceInfo, level)) { + sourceInfo)) { // Silently skip the filtered entry. return B_OK; } @@ -344,7 +337,7 @@ CopyEngine::_Copy(BEntry &source, BEntry &destination, if (destination.IsDirectory()) { if (fEntryFilter && fEntryFilter->ShouldClobberFolder(source, - relativeSourcePath, sourceInfo, level)) { + relativeSourcePath, sourceInfo)) { ret = _RemoveFolder(destination); } else { // Do not overwrite attributes on folders that exist. @@ -381,8 +374,7 @@ CopyEngine::_Copy(BEntry &source, BEntry &destination, ret = dest.InitCheck(); if (ret != B_OK) return ret; - ret = _Copy(entry, dest, level, - cancelSemaphore, copyAttributes); + ret = _Copy(entry, dest, cancelSemaphore, copyAttributes); if (ret != B_OK) return ret; } @@ -470,7 +462,6 @@ CopyEngine::_Copy(BEntry &source, BEntry &destination, destination.SetCreationTime(sourceInfo.st_crtime); } - level--; return B_OK; } diff --git a/src/apps/installer/CopyEngine.h b/src/apps/installer/CopyEngine.h index d4eecdf040..2663ccbeed 100644 --- a/src/apps/installer/CopyEngine.h +++ b/src/apps/installer/CopyEngine.h @@ -39,11 +39,10 @@ public: private: status_t _CollectCopyInfo(const char* source, - int32& level, sem_id cancelSemaphore, - off_t& bytesToCopy, uint64& itemsToCopy); - status_t _Copy(BEntry& source, - BEntry& destination, - int32& level, sem_id cancelSemaphore, + sem_id cancelSemaphore, off_t& bytesToCopy, + uint64& itemsToCopy); + status_t _Copy(BEntry& source, BEntry& destination, + sem_id cancelSemaphore, bool copyAttributes); status_t _CopyData(const BEntry& entry, const BEntry& destination, @@ -118,12 +117,10 @@ public: virtual bool ShouldCopyEntry(const BEntry& entry, const char* path, - const struct stat& statInfo, - int32 level) const = 0; + const struct stat& statInfo) const = 0; virtual bool ShouldClobberFolder(const BEntry& entry, const char* path, - const struct stat& statInfo, - int32 level) const = 0; + const struct stat& statInfo) const = 0; }; diff --git a/src/apps/installer/WorkerThread.cpp b/src/apps/installer/WorkerThread.cpp index 528db15668..1fd607ea75 100644 --- a/src/apps/installer/WorkerThread.cpp +++ b/src/apps/installer/WorkerThread.cpp @@ -113,7 +113,7 @@ public: } virtual bool ShouldCopyEntry(const BEntry& entry, const char* path, - const struct stat& statInfo, int32 level) const + const struct stat& statInfo) const { if (S_ISBLK(statInfo.st_mode) || S_ISCHR(statInfo.st_mode) || S_ISFIFO(statInfo.st_mode) || S_ISSOCK(statInfo.st_mode)) { @@ -138,10 +138,9 @@ public: } virtual bool ShouldClobberFolder(const BEntry& entry, const char* path, - const struct stat& statInfo, int32 level) const + const struct stat& statInfo) const { - if (level == 2 && S_ISDIR(statInfo.st_mode) - && strncmp("system/", path, 7) == 0 + if (S_ISDIR(statInfo.st_mode) && strncmp("system/", path, 7) == 0 && strcmp("system/settings", path) != 0) { // Replace everything in "system" besides "settings" printf("clobbering '%s'.\n", path);