[llvm] [Support] Report EISDIR when opening a directory (PR #79880)
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 15 01:05:29 PDT 2024
================
@@ -1296,6 +1296,36 @@ TEST_F(FileSystemTest, UTF8ToUTF16DirectoryIteration) {
}
#endif
+#ifndef _WIN32
+TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
+ ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
+ ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
+ errc::file_exists);
+
+ std::string Buf(5, '?');
+ Expected<fs::file_t> FD = fs::openNativeFileForRead(TestDirectory);
+ ASSERT_NO_ERROR(errorToErrorCode(FD.takeError()));
+ auto Close = make_scope_exit([&] { fs::closeFile(*FD); });
+ Expected<size_t> BytesRead =
+ fs::readNativeFile(*FD, MutableArrayRef(&*Buf.begin(), Buf.size()));
+ ASSERT_EQ(errorToErrorCode(BytesRead.takeError()), errc::is_a_directory);
+}
+#endif
+
+TEST_F(FileSystemTest, OpenDirectoryAsFileForWrite) {
+ ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
+ ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
+ errc::file_exists);
+
+ int FD;
+ std::error_code EC;
+ EC = fs::openFileForWrite(Twine(TestDirectory), FD);
+ ASSERT_EQ(EC, errc::is_a_directory);
+
+ ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory)));
+ ::close(FD);
----------------
jh7370 wrote:
This test crashes on Windows, because you're attempting to close a file handle that hasn't been opened (more precisely, `FD` is unitialized). I'm surprised this doesn't provoke an assertion or similar on Linux (maybe it does and the pre-merge checks don't have assertions enabled). Either way, this `close` here is incorrect. You probably should have something to make sure `FD` is closed if `openFileForWrite` succeeds (i.e. the test is failing), to prevent a race condition, but the "normal" flow of the test shouldn't attempt to call `close` at all.
https://github.com/llvm/llvm-project/pull/79880
More information about the llvm-commits
mailing list