Skip to content

Commit

Permalink
File iterator: not recursing on pattern no-match. The recursion cause…
Browse files Browse the repository at this point in the history 
…d stack overflow when a folder had too many files. Now there's recursion as deep as the folder hierarchy depth, not as deep as the file count in the entire hierarchy
  • Loading branch information
Nir Bar committed Jan 7, 2025
1 parent d0799c3 commit a23db18
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 80 deletions.
26 changes: 18 additions & 8 deletions src/CaCommon/WixString.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,12 @@ class CWixString

CWixString(CWixString&& other) noexcept
{
_dwCapacity = other._dwCapacity;
_pTokenContext = other._pTokenContext;
_szObfuscated = other._szObfuscated;
_pS = other.Detach();
MoveFrom(other);
}

CWixString& operator=(CWixString&& other) noexcept
{
_dwCapacity = other._dwCapacity;
_pTokenContext = other._pTokenContext;
_szObfuscated = other._szObfuscated;
_pS = other.Detach();
MoveFrom(other);
return *this;
}

Expand All @@ -64,6 +58,22 @@ class CWixString
return *this;
}

void MoveFrom(CWixString& from) noexcept
{
if (this != &from)
{
Release();
_dwCapacity = from._dwCapacity;
_pTokenContext = from._pTokenContext;
_szObfuscated = from._szObfuscated;
_pS = from._pS;

from._pS = nullptr;
from._szObfuscated = nullptr;
from.Release();
}
}

WCHAR& operator[](size_t i)
{
static WCHAR errChr = NULL;
Expand Down
55 changes: 37 additions & 18 deletions src/PanelSwCustomActions/FileEntry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,23 @@ CFileEntry::CFileEntry(const WIN32_FIND_DATA& findData, LPCWSTR szBasePath)
}
}

void CFileEntry::Release() noexcept
{
_dwFileAttributes = INVALID_FILE_ATTRIBUTES;
_ftLastAccessTime.dwLowDateTime = 0;
_ftLastAccessTime.dwHighDateTime = 0;
_ftCreationTime.dwLowDateTime = 0;
_ftCreationTime.dwHighDateTime = 0;
_ftLastWriteTime.dwLowDateTime = 0;
_ftLastWriteTime.dwHighDateTime = 0;

_ulFileSize.QuadPart = 0;
_dwReserved0 = 0;
_szPath.Release();
_szFileName.Release();
_szParentPath.Release();
}

CFileEntry::CFileEntry(const CFileEntry& other)
{
HRESULT hr = S_OK;
Expand Down Expand Up @@ -138,15 +155,7 @@ CFileEntry::CFileEntry(const CFileEntry& other)

CFileEntry::CFileEntry(CFileEntry&& other) noexcept
{
_szPath.Attach(other._szPath.Detach());
_szParentPath.Attach(other._szParentPath.Detach());
_szFileName.Attach(other._szFileName.Detach());
_dwFileAttributes = other._dwFileAttributes;
_ftCreationTime = other._ftCreationTime;
_ftLastAccessTime = other._ftLastAccessTime;
_ftLastWriteTime = other._ftLastWriteTime;
_ulFileSize = other._ulFileSize;
_dwReserved0 = other._dwReserved0;
MoveFrom(other);
}

CFileEntry& CFileEntry::operator=(CFileEntry& other)
Expand Down Expand Up @@ -180,18 +189,28 @@ CFileEntry& CFileEntry::operator=(CFileEntry& other)

CFileEntry& CFileEntry::operator=(CFileEntry&& other) noexcept
{
_szPath.Attach(other._szPath.Detach());
_szParentPath.Attach(other._szParentPath.Detach());
_szFileName.Attach(other._szFileName.Detach());
_dwFileAttributes = other._dwFileAttributes;
_ftCreationTime = other._ftCreationTime;
_ftLastAccessTime = other._ftLastAccessTime;
_ftLastWriteTime = other._ftLastWriteTime;
_ulFileSize = other._ulFileSize;
_dwReserved0 = other._dwReserved0;
MoveFrom(other);
return *this;
}

void CFileEntry::MoveFrom(CFileEntry& other) noexcept
{
if (this != &other)
{
Release();
_szPath.MoveFrom(other._szPath);
_szParentPath.MoveFrom(other._szParentPath);
_szFileName.MoveFrom(other._szFileName);
_dwFileAttributes = other._dwFileAttributes;
_ftCreationTime = other._ftCreationTime;
_ftLastAccessTime = other._ftLastAccessTime;
_ftLastWriteTime = other._ftLastWriteTime;
_ulFileSize = other._ulFileSize;
_dwReserved0 = other._dwReserved0;
other.Release();
}
}

DWORD CFileEntry::Attributes() const
{
return _dwFileAttributes;
Expand Down
3 changes: 3 additions & 0 deletions src/PanelSwCustomActions/FileEntry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ class CFileEntry
CFileEntry& operator=(CFileEntry& other);
CFileEntry& operator=(CFileEntry&& other) noexcept;

void Release() noexcept;
void MoveFrom(CFileEntry& other) noexcept;

// Attribute tests
DWORD Attributes() const;
bool IsValid() const;
Expand Down
128 changes: 77 additions & 51 deletions src/PanelSwCustomActions/FileIterator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ CFileEntry CFileIterator::Find(LPCWSTR szBasePath, LPCWSTR szPattern, bool bRecu
Release();
DWORD dwAttributes = INVALID_FILE_ATTRIBUTES;
CWixString szBasePattern;
bool bSkip = true;
CFileEntry entry = CFileEntry::InvalidFileEntry();

_hrStatus = _szBasePath.Copy(szBasePath);
ExitOnFailure(_hrStatus, "Failed to copy string");
Expand All @@ -45,7 +47,7 @@ CFileEntry CFileIterator::Find(LPCWSTR szBasePath, LPCWSTR szPattern, bool bRecu
_hrStatus = _szPattern.Copy(szPattern);
ExitOnFailure(_hrStatus, "Failed to copy string");
}

_bRecursive = bRecursive;
_bIncludeSelf = bIncludeSelf;

Expand All @@ -62,81 +64,100 @@ CFileEntry CFileIterator::Find(LPCWSTR szBasePath, LPCWSTR szPattern, bool bRecu
ExitOnWin32Error(dwError, _hrStatus, "Failed to find files in '%ls'", szBasePath);
}

return ProcessFindData();
entry = ProcessFindData(&bSkip);
if (bSkip)
{
return Next();
}
return entry;

LExit:
return CFileEntry::InvalidFileEntry();
}

CFileEntry CFileIterator::Next()
{
// Sub dir is enumerating?
if (_pCurrSubDir)
CFileEntry entry = CFileEntry::InvalidFileEntry();

while (true)
{
HRESULT hrSubDir = S_OK;
// First entry of the subdir?
if (_pCurrSubDir->_szBasePath.IsNullOrEmpty())
bool bSkip = false;

// Sub dir is enumerating?
if (_pCurrSubDir)
{
CWixString szSubDirPath;
HRESULT hrSubDir = S_OK;
// First entry of the subdir?
if (_pCurrSubDir->_szBasePath.IsNullOrEmpty())
{
CWixString szSubDirPath;

_hrStatus = szSubDirPath.AppnedFormat(L"%ls\\%ls", (LPCWSTR)_szBasePath, _findData.cFileName);
ExitOnFailure(_hrStatus, "Failed to format fodler path");
_hrStatus = szSubDirPath.AppnedFormat(L"%ls\\%ls", (LPCWSTR)_szBasePath, _findData.cFileName);
ExitOnFailure(_hrStatus, "Failed to format fodler path");

CFileEntry entry = _pCurrSubDir->Find((LPCWSTR)szSubDirPath, _szPattern, _bRecursive, false);
hrSubDir = _pCurrSubDir->Status();
if (SUCCEEDED(hrSubDir))
CFileEntry entry = _pCurrSubDir->Find((LPCWSTR)szSubDirPath, _szPattern, _bRecursive, false);
hrSubDir = _pCurrSubDir->Status();
if (SUCCEEDED(hrSubDir))
{
return entry;
}
}
else
{
return entry;
CFileEntry entry = _pCurrSubDir->Next();
hrSubDir = _pCurrSubDir->Status();
if (SUCCEEDED(hrSubDir))
{
return entry;
}
}
}
else
{
CFileEntry entry = _pCurrSubDir->Next();
hrSubDir = _pCurrSubDir->Status();
if (SUCCEEDED(hrSubDir))

delete _pCurrSubDir;
_pCurrSubDir = nullptr;

// Bad failure
if (hrSubDir != E_NOMOREFILES)
{
return entry;
_hrStatus = hrSubDir;
ExitOnFailure(_hrStatus, "Failed to enumerate entries in %ls\\%ls", (LPCWSTR)_szBasePath, _findData.cFileName);
}
}

delete _pCurrSubDir;
_pCurrSubDir = nullptr;

// Bad failure
if (hrSubDir != E_NOMOREFILES)
if (!::FindNextFile(_hFind, &_findData))
{
_hrStatus = hrSubDir;
ExitOnFailure(_hrStatus, "Failed to enumerate entries in %ls\\%ls", (LPCWSTR)_szBasePath, _findData.cFileName);
_hrStatus = HRESULT_FROM_WIN32(::GetLastError());
if (_hrStatus == E_NOMOREFILES)
{
return CFileEntry::InvalidFileEntry();
}

ExitOnFailure(_hrStatus, "Failed to find next entry in %ls", (LPCWSTR)_szBasePath);
}
}

if (!::FindNextFile(_hFind, &_findData))
{
_hrStatus = HRESULT_FROM_WIN32(::GetLastError());
if (_hrStatus == E_NOMOREFILES)
entry = ProcessFindData(&bSkip);
if (!bSkip)
{
return CFileEntry::InvalidFileEntry();
return entry;
}

ExitOnFailure(_hrStatus, "Failed to find next entry in %ls", (LPCWSTR)_szBasePath);
}

return ProcessFindData();

LExit:
return CFileEntry::InvalidFileEntry();
}

CFileEntry CFileIterator::ProcessFindData()
CFileEntry CFileIterator::ProcessFindData(bool* pbSkip)
{
*pbSkip = false;

if (_bIncludeSelf && (wcscmp(_findData.cFileName, L".") == 0))
{
CFileEntry entry(_findData, (LPCWSTR)_szBasePath);
return ProcessEntry(entry);
return ProcessEntry(entry, pbSkip);
}
if ((wcscmp(_findData.cFileName, L".") == 0) || (wcscmp(_findData.cFileName, L"..") == 0))
{
return Next();
*pbSkip = true;
return CFileEntry::InvalidFileEntry();
}

// File?
Expand All @@ -145,37 +166,40 @@ CFileEntry CFileIterator::ProcessFindData()
// Skip files that don't match the wildcard pattern
if (_szPattern.IsNullOrEmpty() || !::PathMatchSpec(_findData.cFileName, _szPattern))
{
return Next();
*pbSkip = true;
return CFileEntry::InvalidFileEntry();
}

CFileEntry entry(_findData, (LPCWSTR)_szBasePath);
return ProcessEntry(entry);
return ProcessEntry(entry, pbSkip);
}
// Skip symlink folders
if (CFileEntry::IsSymlink(_findData.dwFileAttributes, _findData.dwReserved0))
{
// Skip files that don't match the wildcard pattern
if (_szPattern.IsNullOrEmpty() || !::PathMatchSpec(_findData.cFileName, _szPattern))
{
return Next();
*pbSkip = true;
return CFileEntry::InvalidFileEntry();
}

WcaLog(LOGLEVEL::LOGMSG_STANDARD, "Skipping files under symbolic link folder '%ls\\%ls'", (LPCWSTR)_szBasePath, _findData.cFileName);
CFileEntry entry(_findData, (LPCWSTR)_szBasePath);
return ProcessEntry(entry);
return ProcessEntry(entry, pbSkip);
}
// Skip mount folders
if (CFileEntry::IsMountPoint(_findData.dwFileAttributes, _findData.dwReserved0))
{
// Skip files that don't match the wildcard pattern
if (_szPattern.IsNullOrEmpty() || !::PathMatchSpec(_findData.cFileName, _szPattern))
{
return Next();
*pbSkip = true;
return CFileEntry::InvalidFileEntry();
}

WcaLog(LOGLEVEL::LOGMSG_STANDARD, "Skipping files under mount point folder '%ls\\%ls'", (LPCWSTR)_szBasePath, _findData.cFileName);
CFileEntry entry(_findData, (LPCWSTR)_szBasePath);
return ProcessEntry(entry);
return ProcessEntry(entry, pbSkip);
}

// Return the subfolder name too?
Expand All @@ -189,16 +213,17 @@ CFileEntry CFileIterator::ProcessFindData()
}

CFileEntry entry(_findData, (LPCWSTR)_szBasePath);
return ProcessEntry(entry);
return ProcessEntry(entry, pbSkip);
}

return Next();
*pbSkip = true;
return CFileEntry::InvalidFileEntry();

LExit:
return CFileEntry::InvalidFileEntry();
}

CFileEntry CFileIterator::ProcessEntry(CFileEntry entry)
CFileEntry CFileIterator::ProcessEntry(CFileEntry entry, bool* pbSkip)
{
if (entry.IsValid())
{
Expand All @@ -212,7 +237,8 @@ CFileEntry CFileIterator::ProcessEntry(CFileEntry entry)
delete _pCurrSubDir;
_pCurrSubDir = nullptr;

return Next();
*pbSkip = true;
return CFileEntry::InvalidFileEntry();
}

if (FAILED(_pCurrSubDir->Status()))
Expand Down
4 changes: 2 additions & 2 deletions src/PanelSwCustomActions/FileIterator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ class CFileIterator
private:

CFileEntry Find(LPCWSTR szBasePath, LPCWSTR szPattern, bool bRecursive, bool bIncludeSelf);
CFileEntry ProcessFindData();
CFileEntry ProcessEntry(CFileEntry entry);
CFileEntry ProcessFindData(bool * pbSkip);
CFileEntry ProcessEntry(CFileEntry entry, bool* pbSkip);

HANDLE _hFind = INVALID_HANDLE_VALUE;
WIN32_FIND_DATA _findData = {};
Expand Down
2 changes: 1 addition & 1 deletion src/TidyBuild.custom.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<Project ToolsVersion="16.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildThisFileDirectory)TidyBuild.user.props" Condition="Exists('$(MSBuildThisFileDirectory)TidyBuild.user.props')"/>
<PropertyGroup>
<FullVersion>3.23.0</FullVersion>
<FullVersion>3.23.1</FullVersion>
<FullVersion Condition=" '$(GITHUB_RUN_NUMBER)'!='' ">$(FullVersion).$(GITHUB_RUN_NUMBER)</FullVersion>
<ProductName>PanelSwWixExtension</ProductName>
<Manufacturer>Panel::Software</Manufacturer>
Expand Down

0 comments on commit a23db18

Please sign in to comment.