[libcxx-commits] [PATCH] D92769: [libc++] [P1164] [C++20] Make fs::create_directory() error if there is already a non-directory.

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

In D92769#2445438 <https://reviews.llvm.org/D92769#2445438>, @curdeius wrote:

> In D92769#2444967 <https://reviews.llvm.org/D92769#2444967>, @mstorsjo wrote:
>
>> Note that P1164 <https://reviews.llvm.org/P1164> talks about both `create_directory` and `create_directories`, while this fix only covers `create_directory`.
>
> Indeed, but it doesn't change the behaviour of `create_directories` per se because it calls `create_directory` for every part of the path.

Ah, right.

After revisiting the issue I was, it was that while `create_directories` does report failures, it reports a different error code than expected - at least with the windows filesystem APIs. (But maybe the testcase I'm running, one picked from MS STL's testsuite, is overly strict on checking the specific error codes?) I'll comment inline to point out and explain the issue.



================
Comment at: libcxx/src/filesystem/operations.cpp:844
   }
   return __create_directory(p, ec);
 }
----------------
If parent does exist, but isn't a directory, the check `if (not exists(parent_st))` above won't trigger, so it won't hit the case of calling `__create_directory("parent_which_is_file")`, but will instead call `__create_directory("parent_which_is_file/subdir")`. That call doesn't return the error code corresponding to `EEXIST`, but gives an error corresponding to `no_such_file_or_directory`/`ENOENT` instead.

By extending the check above into `if (not exists(parent_st)) { ... } else if (!is_directory(parent)) return  err.report(errc::file_exists);`, one would get the more expected error code here too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92769/new/

https://reviews.llvm.org/D92769