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

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius marked an inline comment as done.
curdeius added inline comments.


================
Comment at: libcxx/src/filesystem/operations.cpp:844
   }
   return __create_directory(p, ec);
 }
----------------
mstorsjo wrote:
> 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.
Moved discussion to D93026.


================
Comment at: libcxx/src/filesystem/operations.cpp:844
   }
   return __create_directory(p, ec);
 }
----------------
curdeius wrote:
> mstorsjo wrote:
> > 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.
> Moved discussion to D93026.
> 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