Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

FindNextFile returns error in Linux on bad symlink causing short Dir read #1675

Closed
wants to merge 2 commits into from

1 participant

@dragonflight

In the linux implementation of FindNextFile in linux/XFileUtils.cpp, FindNextFile will fail (return error) if the "next" file is a broken symbolic link. this has the (unintended I'm sure) effect of cutting directories entries short. If the first entry in directory is a broken symbolic link then FindFirstFile returns a handle, gives no indication of a problem and returns a partially populated LPWIN32_FIND_DATA structure.

Not being a Windows programmer and the documentation is not clear on what windows will do in this situation, someone need to check on it. If Windows has the same problem then this fix should probably be abandoned and a wrapper around FindNextFile and FindFirstFile created to do the same thing (but for all implementations presumably)

Note: this will also occur on insufficient permissions getting to a target of a symlink

dragonflight added some commits
@dragonflight dragonflight Modify FindFirstFile and FindNextFile to always return a valid file o…
…r end of directory

FindFirstFile could have returned a partially populated WIN32_FIND_DATA structure
and FindNextFile could return false in the middle of a directory due to
a broken symlink.
e538970
@dragonflight dragonflight clean up indenting from previous fix bdd0ad2
@ghost

rebase.

@dragonflight

It's a fact I'm an idiot - I checked against the wrong branch before I posted. I also misinterpreted why the stat - thinking it was to prevent having to check file validity later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 25, 2012
  1. @dragonflight

    Modify FindFirstFile and FindNextFile to always return a valid file o…

    dragonflight authored
    …r end of directory
    
    FindFirstFile could have returned a partially populated WIN32_FIND_DATA structure
    and FindNextFile could return false in the middle of a directory due to
    a broken symlink.
  2. @dragonflight
This page is out of date. Refresh to see the latest.
Showing with 33 additions and 35 deletions.
  1. +33 −35 xbmc/linux/XFileUtils.cpp
View
68 xbmc/linux/XFileUtils.cpp
@@ -126,14 +126,11 @@ HANDLE FindFirstFile(LPCSTR szPath,LPWIN32_FIND_DATA lpFindData)
}
free(namelist);
- if (pHandle->m_FindFileResults.size() == 0)
+ if (pHandle->m_FindFileResults.size() == 0 || !FindNextFile(pHandle, lpFindData) )
{
delete pHandle;
return INVALID_HANDLE_VALUE;
}
-
- FindNextFile(pHandle, lpFindData);
-
return pHandle;
}
@@ -142,49 +139,50 @@ BOOL FindNextFile(HANDLE hHandle, LPWIN32_FIND_DATA lpFindData)
if (lpFindData == NULL || hHandle == NULL || hHandle->GetType() != CXHandle::HND_FIND_FILE)
return FALSE;
- if ((unsigned int) hHandle->m_nFindFileIterator >= hHandle->m_FindFileResults.size())
- return FALSE;
+ while ((unsigned int) hHandle->m_nFindFileIterator < hHandle->m_FindFileResults.size())
+ {
+ CStdString strFileName = hHandle->m_FindFileResults[hHandle->m_nFindFileIterator++];
+ CStdString strFileNameTest = hHandle->m_FindFileDir + strFileName;
- CStdString strFileName = hHandle->m_FindFileResults[hHandle->m_nFindFileIterator++];
- CStdString strFileNameTest = hHandle->m_FindFileDir + strFileName;
+ if (IsAliasShortcut(strFileNameTest))
+ TranslateAliasShortcut(strFileNameTest);
- if (IsAliasShortcut(strFileNameTest))
- TranslateAliasShortcut(strFileNameTest);
+ struct stat64 fileStat;
+ memset(&fileStat, 0, sizeof(fileStat));
- struct stat64 fileStat;
- memset(&fileStat, 0, sizeof(fileStat));
-
- if (stat64(strFileNameTest, &fileStat) == -1)
- return FALSE;
+ if (stat64(strFileNameTest, &fileStat) == -1)
+ continue;
- bool bIsDir = false;
- if (S_ISDIR(fileStat.st_mode))
- {
- bIsDir = true;
- }
+ bool bIsDir = false;
+ if (S_ISDIR(fileStat.st_mode))
+ {
+ bIsDir = true;
+ }
- memset(lpFindData,0,sizeof(WIN32_FIND_DATA));
+ memset(lpFindData,0,sizeof(WIN32_FIND_DATA));
- lpFindData->dwFileAttributes = FILE_ATTRIBUTE_NORMAL;
- strcpy(lpFindData->cFileName, strFileName.c_str());
+ lpFindData->dwFileAttributes = FILE_ATTRIBUTE_NORMAL;
+ strcpy(lpFindData->cFileName, strFileName.c_str());
- if (bIsDir)
- lpFindData->dwFileAttributes |= FILE_ATTRIBUTE_DIRECTORY;
+ if (bIsDir)
+ lpFindData->dwFileAttributes |= FILE_ATTRIBUTE_DIRECTORY;
- if (strFileName[0] == '.')
- lpFindData->dwFileAttributes |= FILE_ATTRIBUTE_HIDDEN;
+ if (strFileName[0] == '.')
+ lpFindData->dwFileAttributes |= FILE_ATTRIBUTE_HIDDEN;
- if (access(strFileName, R_OK) == 0 && access(strFileName, W_OK) != 0)
- lpFindData->dwFileAttributes |= FILE_ATTRIBUTE_READONLY;
+ if (access(strFileName, R_OK) == 0 && access(strFileName, W_OK) != 0)
+ lpFindData->dwFileAttributes |= FILE_ATTRIBUTE_READONLY;
- TimeTToFileTime(fileStat.st_ctime, &lpFindData->ftCreationTime);
- TimeTToFileTime(fileStat.st_atime, &lpFindData->ftLastAccessTime);
- TimeTToFileTime(fileStat.st_mtime, &lpFindData->ftLastWriteTime);
+ TimeTToFileTime(fileStat.st_ctime, &lpFindData->ftCreationTime);
+ TimeTToFileTime(fileStat.st_atime, &lpFindData->ftLastAccessTime);
+ TimeTToFileTime(fileStat.st_mtime, &lpFindData->ftLastWriteTime);
- lpFindData->nFileSizeHigh = (DWORD)(fileStat.st_size >> 32);
- lpFindData->nFileSizeLow = (DWORD)fileStat.st_size;
+ lpFindData->nFileSizeHigh = (DWORD)(fileStat.st_size >> 32);
+ lpFindData->nFileSizeLow = (DWORD)fileStat.st_size;
- return TRUE;
+ return TRUE;
+ }
+ return FALSE;
}
BOOL FindClose(HANDLE hFindFile)
Something went wrong with that request. Please try again.