directory_iterator: Skip entry if there was an error creating it.

[jpd002]

Hi!

So, I've been trying to debug some issues we have in our game scanner in Play! on Android. We are using ghc_filesystem to scan the filesystem for files that can be used by the emulator (code here).

One issue I've identified is that inside the directory_iterator::impl::increment function, readdir seems to return an entry that can't be lstat'ed (returns -1). I'm still trying to see what's weird about that entry, but it's causing the directory_iterator constructor to throw an exception and aborting the scanning process for that directory (which happens to be the root of the sdcard filesystem in my case).

The PR here is a workaround for that problem, but I'm not sure this is the proper fix. I think the expected behavior would be to allow a directory_entry to be created with that entry without an error, even though other operations, such as fs::is_directory would fail (which would be ok).

Lemme know what you think!

[gulrak]

Sorry, for my late reaction, I'm not feeling that well this week, but nothing dramatic.

Still, I had a quick look at the linked code that is using the iterator and my first idea was something along these lines:

void ScanBootables(const fs::path& parentPath, bool recursive)
{
    std::error_code ec;
    for(auto pathIterator = fs::directory_iterator(parentPath, ec);
        pathIterator != fs::directory_iterator(); pathIterator.increment(ec))
    {
        if(!ec) {
            auto& path = pathIterator->path();
            if(recursive && fs::is_directory(path, ec))
            {
                ScanBootables(path, recursive);
                continue;
            }
            // not sure if this needs a try/catch block: 
            TryRegisteringBootable(path);
        }
    }
}

This is a std-conforming exception-less way of iterating the directory and should work just fine. (I wasn't sure if TryRegisteringBootable(path) would still need exception handling, so that might be missing.)

Without bigger changes the PR would hide errors that other use-cases might want to see. I admit that the directory_iterator still could be improved, e.g. defering the refresh() until the attributes are needed (e.g. for iter->file_size()), to give better performance. It's on my list, but I need to have time to get to that.

[jpd002]

Ah, that seems like sensible alternative. I'll give it a spin and tell you how it fares!

Thanks!

[jpd002]

Seems to do the trick! Thanks for the suggestion!

[gulrak]

I'm glad I could help!