misterdjules
commented
10 years ago @saghul @tjfontaine Reopening a new PR because GitHub wouldn't let me reopen #1521.
The code has been updated to follow @saghul's design closely.
There are still a few of things that are unclear or missing:
- As @saghul mentioned:
Regardless of the API accepting a single entry or an array of them, space for the name needs to be allocated, maybe have an extra function to help with that?. Do we want something similar to handles' allocation callback? - Regarding @piscisaureus's comment about GetFileAttributesW/FindFirstFileW being inherently racey, using only
FindFirstFileWon an entry that is not a directory would return UV_ENOENT, which I find a bit counter-intuitive and inconsistent. @piscisaureus: before looking into the approach you recommended ("Open the directory, Use GetFileInformationByHandle to read the attributes. Bail out and close if it's not a directory, otherwise start enumerating entries (using a different call).", I'd like to make sure I understood it correctly. Do you mean that we could useCreateFilefirst, thenGetFileInformationByHandleto check if it's a directory, and then use lower-level APIs (notFindFirstFile/FindNextFile) to iterate through all entries of theHANDLEreturned byCreateFile? It seems to be quite more complicated than usingFindFirstFile/FindNextFile, but it could be because I'm not familiar with these others lower-level APIs. - Regarding the following question: "What happens if we create 2 readdir requests for the same directory? On Unix we should be fine because we'd use readdir_r, but how about windows?", I've done some research to determine how to answer this question, but I'm not done yet.
Currently, all tests pass on all supported platforms. I've also done a lot of tests with directories containing more than 1 million entries, and the memory footprint stays constant and low.
Hopefully this is a step in the right direction!
Thank you,
saghul
Tested on Linux, MacOS X, SmartOS and Windows.
Fixes #1430.