3

I wrote a simple code to do some operation on every file in every folder (subfolders). It's perfectly works until the path comes with 'SPACE ' character program crashs and INVALID_HANDLE_VALUE has been called. This is function:

int dirListFiles(char* startDir)
{
    HANDLE hFind;
    WIN32_FIND_DATAA  wfd;
    char path[MAX_PATH];

    sprintf(path, "%s\\*", startDir);

    std::string fileName;
    std::string s_path = startDir;
    std::string fullPath;

    fprintf(stdout, "In Directory \"%s\"\n\n", startDir);

    if ((hFind = FindFirstFileA(path, &wfd)) == INVALID_HANDLE_VALUE)
    {
        printf("FindFirstFIle failed on path = \"%s\"\n", path);
        abort();
    }

    BOOL cont = TRUE;
    while (cont == TRUE)
    {
        if ((strncmp(".", wfd.cFileName, 1) != 0) && (strncmp("..", wfd.cFileName, 2) != 0))
        {
            if (wfd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
            {
                sprintf(path, "%s\\%s", startDir, wfd.cFileName);
                dirListFiles(path);
            }
        else
        {
            fileName = wfd.cFileName;
            fullPath = s_path + "\\" + fileName;    

            std::string fileExt = PathFindExtension(fullPath.c_str());
            if (fileExt == ".cpp")
            {
                ... Some operation on file
            }
        }
    }
    cont = FindNextFile(hFind, &wfd);
}

FindClose(hFind);

For example, If FindNextFile wants to Open Program Files (x86) which has space between file name cause error and program exit. What Can I do for supporting spaces? What Is Problem?

Ron
  • 14,674
  • 4
  • 34
  • 47
Ali Sepehri-Amin
  • 493
  • 1
  • 6
  • 18
  • You should call `GetLastError` and inspect the value returned. – user7860670 Oct 14 '17 at 13:46
  • nothing need do. space is usual character. and why somebody still use `FindFirstFileA` instead `FindFirstFileEx` how minimum ? – RbMm Oct 14 '17 at 14:09
  • @RbMm Does it make a difference in this situation? – Ali Sepehri-Amin Oct 14 '17 at 14:10
  • 1
    in performance difference huge. – RbMm Oct 14 '17 at 14:11
  • 1
    code in all case have many mistakes. for example you fail with long paths (> MAX_PATH), etc. – RbMm Oct 14 '17 at 14:18
  • I think it may be wrong before hand, and perhaps command line arguments have split on space. – mksteve Oct 14 '17 at 14:24
  • first of all very common mistake - how file paths is processed. you need at begin once allocate `0x8000` *wchar* length buffer and pass it to function with current position in buffer to where need append \ and mask. and of course never use *A* api, only *W*, – RbMm Oct 14 '17 at 14:28
  • @RbMm Yes I figured out the main problem is `string` and `char*` it should be allocated and `wstring` or(`LPCWSTR`) , `wchar_t` instead of those. – Ali Sepehri-Amin Oct 14 '17 at 14:30
  • @mksteve No I initial value in the source. – Ali Sepehri-Amin Oct 14 '17 at 14:31
  • 2
    @AliSepehri-Amin - you need only once allocate maximum path length buffer which is `0x8000` *wchar* (64kb). after this you never need allocate any names. you simply need append file names to current position in buffer – RbMm Oct 14 '17 at 14:33
  • Maybe the path doesn't contain a space character, but rather a Unicode code point, that cannot be represented in the ANSI codepage you decided to use. Incidentally, why aren't you using Unicode? Or a [recursive_directory_iterator](https://learn.microsoft.com/en-us/cpp/standard-library/recursive-directory-iterator-class) for that matter? – IInspectable Oct 14 '17 at 14:50
  • Should I Use `HealAlloc` or `malloc`? How? – Ali Sepehri-Amin Oct 14 '17 at 14:56
  • 2
    @AliSepehri-Amin - how you think - what different ? you can simply allocate it in stack as local var before first function call `WCHAR buf[0x8000];` – RbMm Oct 14 '17 at 14:59
  • 1
    `WIN32_FIND_DATA` also much better once declare before recursive function call and pass it as pointer, instead declare it inside function (too large structure for recursive call) – RbMm Oct 14 '17 at 15:01
  • 1
    If you need to support paths longer than `MAX_PATH` or unusual names, then begin by normalizing the input path to an extended path. Ensure the input path ends with a backslash and call `GetFullPathName`. Then handle the various input cases -- paths that are already extended with `L"\\\\?\\"`; local-device paths that start with `L"\\\\.\\"` for which you replace "." with "?"; logical-drive-letter paths that need to be prefixed by `L"\\\\?\\"`; and UNC paths that need to be written as `L"\\\\?\\UNC\\server\\share\\<...>"`. – Eryk Sun Oct 14 '17 at 18:59

1 Answers1

1

Space is legal character in directory and file names.

First I propose to modify slightly your code:

if ((hFind = FindFirstFileA(path, &wfd)) == INVALID_HANDLE_VALUE)
{
    printf("FindFirstFIle failed on path = \"%s\". Error %d\n", path, GetLastError());
    return 0; // I think you shouldn't abort on error, just skip this dir.
}

Now check error codes reported by your program.

For some paths I have got error #5 (access denied). Examples:

c:\Program Files (x86)\Google\CrashReports\*
c:\ProgramData\Microsoft\Windows Defender\Clean Store\*
c:\Windows\System32\config\*

Got two cases with code #123 (Invalid name) for path names unmanageable by FindFirstFileA. To correct this behavior it would be better to use wide version of function FindFirstFileW. See both answers for c++ folder only search. For new Windows applications you should use wide version of API, converting with MultiByteToWideChar and WideCharToMultiByte if needed.

You have also logic error. Code skips all directories and files starting with dot.

Daniel Sęk
  • 2,504
  • 1
  • 8
  • 17
  • 4
    Assuming that error code `ERROR_INVALID_NAME` is raised, when a path name cannot be represented using the current ANSI code page, that can be easily addressed by using Unicode. Even if it isn't, Unicode should *always* be used with file I/O. It's safer, faster, and (potentially) relaxes many length limitations. – IInspectable Oct 14 '17 at 17:37