Skip to content

Conversation

@minerscale
Copy link

Fixes #4.

Hopefully Now There Are No Bugs Left Here What So Ever™

Copy link
Owner

@OneSadCookie OneSadCookie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

casepath is static, and compiled out for _WIN32 targets. I noticed it's actually possible to have case-sensitive file systems on Windows (eg. WSL mounts), so maybe this code should always exist and run, not be static, and be declared in the header?

If we made that change, then at least we could write tests for it...

The other thing that concerns me about this PR is that it's pretty difficult to know whether the directory is correctly closed on all paths. No particular reason to believe this is worse in that regard than what's already there, but I'd rather not inadvertently introduce a new bug there. Any ideas how to test that, or to rewrite the function to be clearer that it closes all directories it opens, gratefully received.

@minerscale
Copy link
Author

minerscale commented Dec 31, 2023

Interesting note about WSL. I will note that in my use case I am checking #ifndef _WIN32 before using casepath.

I came up with a proof for there always being no open directories upon function returns:

  1. Both branches of the initial directory opening open exactly one directory. This happens once.
  2. In the rest of the function whenever a directory is opened the immediately preceding line closes the previous directory ensuring that there is at most one directory open at any given time.
  3. There are three return points of this function. The first one will only return if the folder is not open. The other two check if the directory is open with if(d) and if it is it will close it if it is open before returning. Together they cover all return points of the function.
  4. Since the latest directory is closed before exiting [3], and there cannot be more than one open directory [2]. It follows that all directories are closed before function return.

@minerscale minerscale closed this Dec 31, 2023
@minerscale minerscale reopened this Dec 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

casepath return value is true when a file (but not parent directory) doesn't exist.

2 participants