1
votes

I'm trying to use the WINAPI functions FindFirstFile and FindNextFile. However, I'm experiencing some issues.

When I first call the function FindFirstFile it works fine. I've got a valid handler and the first folder/file name is properly populated inside the WIN32_FIND_DATA structure. No error is found with GetLastError.

Then, I call FindNextFile which returns true as there are more folders in the directory I'm scanning. But I can't retrieve the next folder/file name and GetLastError returns 123 (0x7B) ERROR_INVALID_NAME. I'm a little confused as it says in the official documentation that if an error occurs it should return 0.

https://msdn.microsoft.com/en-us/library/windows/desktop/aa364428(v=vs.85).aspx

Return value

If the function succeeds, the return value is nonzero and the lpFindFileData parameter contains information about the next file or directory found. If the function fails, the return value is zero and the contents of lpFindFileData are indeterminate. To get extended error information, call the GetLastError function. If the function fails because no more matching files can be found, the GetLastError function returns ERROR_NO_MORE_FILES.

I'm using .NET 4.5.1 and Visual Studio 2013 on Windows 7 x64. Here is a sample code.

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)]
public struct WIN32_FIND_DATA
{
    public uint dwFileAttributes;
    public System.Runtime.InteropServices.ComTypes.FILETIME ftCreationTime;
    public System.Runtime.InteropServices.ComTypes.FILETIME ftLastAccessTime;
    public System.Runtime.InteropServices.ComTypes.FILETIME ftLastWriteTime;
    public uint nFileSizeHigh;
    public uint nFileSizeLow;
    public uint dwReserved0;
    public uint dwReserved1;
    [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 260)]
    public string cFileName;
    [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 14)]
    public string cAlternateFileName;
}

...

    [DllImport("Kernel32.dll", EntryPoint = "FindFirstFile", SetLastError = true)]
    public static extern IntPtr FindFirstFile(string lpFileName, ref WIN32_FIND_DATA lpFindFileData);

    [return: MarshalAs(UnmanagedType.Bool)]
    [DllImport("Kernel32.dll", EntryPoint = "FindFirstFile", SetLastError = true)]
    public static extern bool FindNextFile(IntPtr hFindFile, ref WIN32_FIND_DATA lpFindFileData);

    [return: MarshalAs(UnmanagedType.Bool)]
    [DllImport("Kernel32.dll", EntryPoint = "FindClose", SetLastError = true)]
    public static extern bool FindClose(IntPtr hFindFile);

...

public static void Test()
{
    WIN32_FIND_DATA metaDataFile = new WIN32_FIND_DATA();

    IntPtr nextHandle = FileScanner.FindFirstFile("C:\\*", ref metaDataFile);
    Console.WriteLine(Marshal.GetLastWin32Error()); // This equals 0x0 ERROR_SUCCESS
    Console.WriteLine(metaDataFile.cFileName); // This equals $Recycle.Bin

    /* Check invalid handler */
    if (nextHandle != new IntPtr(-1L))
    {
        bool moreFiles = true;
        while (moreFiles)
        {
            moreFiles = FileScanner.FindNextFile(nextHandle, ref metaDataFile);
            Console.WriteLine(Marshal.GetLastWin32Error()); // This equals 0x7B ERROR_INVALID_NAME
            Console.WriteLine(metaDataFile.cFileName); // This equals $Recycle.Bin and this value never change. 
        }
    
    FindClose(nextHandle);
    }
}

For some reason, moreFiles is always true and GetLastError returns ERROR_INVALID_NAME ...

If you need any details please ask me. Any help would really be appreciated !

1
Don't call GetLastError() unless something actually fails (i.e. returns a documented failure code), otherwise the results are meaningless.Jonathan Potter
When reinventing the wheel, make sure that what you have come up with actually does resemble a wheel. If that is too tedious, go with System.IO.Directory.EnumerateDirectories or System.IO.Directory.EnumerateFiles instead. Complete functionality is documented at System.IO.Directory.IInspectable

1 Answers

4
votes

Only call Marshal.GetLastWin32Error is the API call reports failure. In the case of FindNextFile it does so by returning false. You are checking the value returned by Marshal.GetLastWin32Error indiscriminately.

The documentation makes this clear when it tells you how the functions indicate failure. You even linked the text. But you said:

I'm a little confused as it says in the official documentation that if an error occurs it should return 0.

That's right. So check the return value against 0. In the case of a BOOL marshalled as C# bool, that means that the function returns false if it fails. But you simply ignored the return value and tested the value returned by Marshal.GetLastWin32Error(), something altogether different.

The code should be more like this:

public static void Test()
{
    WIN32_FIND_DATA fd = new WIN32_FIND_DATA();

    IntPtr findHandle = FileScanner.FindFirstFile("C:\\*", ref fd);
    if (findHandle == INVALID_HANDLE_VALUE)
        throw new Win32Exception();

    do
    {
        Console.WriteLine(fd.cFileName);
    } while (FileScanner.FindNextFile(findHandle, ref fd));

    // you might check that Marshal.GetLastWin32Error() returns ERROR_NO_MORE_FILES
    // at this point, otherwise the enumeration failed abnormally

    if (!FindClose(findHandle))
        throw new Win32Exception();
}

Your other problem, and it's what hurting you most, is your p/invoke declarations. Look closely at this one:

[return: MarshalAs(UnmanagedType.Bool)]
[DllImport("Kernel32.dll", EntryPoint = "FindFirstFile", SetLastError = true)]
public static extern bool FindNextFile(IntPtr hFindFile,
    ref WIN32_FIND_DATA lpFindFileData);

The EntryPoint is not correct. So you are actually calling FindFirstFile instead of FindNextFile and it's hardly surprising that fails.

Specifying the EntryPoint when you don't need to is simply asking for trouble. And you've fallen right into the trap. I'd declare these imports like so:

[DllImport("Kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
public static extern IntPtr FindFirstFile(string lpFileName,
    ref WIN32_FIND_DATA lpFindFileData);

[DllImport("Kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
public static extern bool FindNextFile(IntPtr hFindFile,
    ref WIN32_FIND_DATA lpFindFileData);

[DllImport("Kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
public static extern bool FindClose(IntPtr hFindFile);

Note that there's no need for the return attribute since UnmanagedType.Bool is the default.

And then you'd need to change the CharSet on the struct to be CharSet.Unicode to match. There's no point at all in opting for ANSI here.

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
public struct WIN32_FIND_DATA
{
    ....
}

Finally, all of this code seems to me to be pointless. What's wrong with Directory.EnumerateFiles and Directory.EnumerateDirectories?