[llvm] r224046 - Remove a convoluted way of calling close by moving the call to the only caller.
Rafael Espindola
rafael.espindola at gmail.com
Thu Dec 11 12:12:56 PST 2014
Author: rafael
Date: Thu Dec 11 14:12:55 2014
New Revision: 224046
URL: http://llvm.org/viewvc/llvm-project?rev=224046&view=rev
Log:
Remove a convoluted way of calling close by moving the call to the only caller.
As a bonus we can actually check the return value.
Modified:
llvm/trunk/include/llvm/Support/FileSystem.h
llvm/trunk/lib/Support/FileOutputBuffer.cpp
llvm/trunk/lib/Support/MemoryBuffer.cpp
llvm/trunk/lib/Support/Unix/Path.inc
llvm/trunk/lib/Support/Windows/Path.inc
llvm/trunk/unittests/Support/Path.cpp
Modified: llvm/trunk/include/llvm/Support/FileSystem.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=224046&r1=224045&r2=224046&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/FileSystem.h Thu Dec 11 14:12:55 2014
@@ -637,7 +637,6 @@ public:
private:
/// Platform-specific mapping state.
- mapmode Mode;
uint64_t Size;
void *Mapping;
#ifdef LLVM_ON_WIN32
@@ -646,19 +645,14 @@ private:
void *FileMappingHandle;
#endif
- std::error_code init(int FD, bool CloseFD, uint64_t Offset);
+ std::error_code init(int FD, uint64_t Offset, mapmode Mode);
public:
- typedef char char_type;
-
- mapped_file_region(mapped_file_region&&);
- mapped_file_region &operator =(mapped_file_region&&);
-
/// \param fd An open file descriptor to map. mapped_file_region takes
/// ownership if closefd is true. It must have been opended in the correct
/// mode.
- mapped_file_region(int fd, bool closefd, mapmode mode, uint64_t length,
- uint64_t offset, std::error_code &ec);
+ mapped_file_region(int fd, mapmode mode, uint64_t length, uint64_t offset,
+ std::error_code &ec);
~mapped_file_region();
Modified: llvm/trunk/lib/Support/FileOutputBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FileOutputBuffer.cpp?rev=224046&r1=224045&r2=224046&view=diff
==============================================================================
--- llvm/trunk/lib/Support/FileOutputBuffer.cpp (original)
+++ llvm/trunk/lib/Support/FileOutputBuffer.cpp Thu Dec 11 14:12:55 2014
@@ -18,6 +18,12 @@
#include "llvm/Support/raw_ostream.h"
#include <system_error>
+#if !defined(_MSC_VER) && !defined(__MINGW32__)
+#include <unistd.h>
+#else
+#include <io.h>
+#endif
+
using llvm::sys::fs::mapped_file_region;
namespace llvm {
@@ -72,9 +78,12 @@ FileOutputBuffer::create(StringRef FileP
return EC;
auto MappedFile = llvm::make_unique<mapped_file_region>(
- FD, true, mapped_file_region::readwrite, Size, 0, EC);
+ FD, mapped_file_region::readwrite, Size, 0, EC);
+ int Ret = close(FD);
if (EC)
return EC;
+ if (Ret)
+ return std::error_code(errno, std::generic_category());
Result.reset(
new FileOutputBuffer(std::move(MappedFile), FilePath, TempFilePath));
Modified: llvm/trunk/lib/Support/MemoryBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/MemoryBuffer.cpp?rev=224046&r1=224045&r2=224046&view=diff
==============================================================================
--- llvm/trunk/lib/Support/MemoryBuffer.cpp (original)
+++ llvm/trunk/lib/Support/MemoryBuffer.cpp Thu Dec 11 14:12:55 2014
@@ -204,7 +204,7 @@ class MemoryBufferMMapFile : public Memo
public:
MemoryBufferMMapFile(bool RequiresNullTerminator, int FD, uint64_t Len,
uint64_t Offset, std::error_code EC)
- : MFR(FD, false, sys::fs::mapped_file_region::readonly,
+ : MFR(FD, sys::fs::mapped_file_region::readonly,
getLegalMapSize(Len, Offset), getLegalMapOffset(Offset), EC) {
if (!EC) {
const char *Start = getStart(Len, Offset);
Modified: llvm/trunk/lib/Support/Unix/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=224046&r1=224045&r2=224046&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Path.inc (original)
+++ llvm/trunk/lib/Support/Unix/Path.inc Thu Dec 11 14:12:55 2014
@@ -62,31 +62,6 @@
using namespace llvm;
-namespace {
- /// This class automatically closes the given file descriptor when it goes out
- /// of scope. You can take back explicit ownership of the file descriptor by
- /// calling take(). The destructor does not verify that close was successful.
- /// Therefore, never allow this class to call close on a file descriptor that
- /// has been read from or written to.
- struct AutoFD {
- int FileDescriptor;
-
- AutoFD(int fd) : FileDescriptor(fd) {}
- ~AutoFD() {
- if (FileDescriptor >= 0)
- ::close(FileDescriptor);
- }
-
- int take() {
- int ret = FileDescriptor;
- FileDescriptor = -1;
- return ret;
- }
-
- operator int() const {return FileDescriptor;}
- };
-}
-
namespace llvm {
namespace sys {
namespace fs {
@@ -440,11 +415,8 @@ std::error_code setLastModificationAndAc
#endif
}
-std::error_code mapped_file_region::init(int FD, bool CloseFD, uint64_t Offset) {
- AutoFD ScopedFD(FD);
- if (!CloseFD)
- ScopedFD.take();
-
+std::error_code mapped_file_region::init(int FD, uint64_t Offset,
+ mapmode Mode) {
// Figure out how large the file is.
struct stat FileInfo;
if (fstat(FD, &FileInfo) == -1)
@@ -470,22 +442,16 @@ std::error_code mapped_file_region::init
return std::error_code();
}
-mapped_file_region::mapped_file_region(int fd,
- bool closefd,
- mapmode mode,
- uint64_t length,
- uint64_t offset,
- std::error_code &ec)
- : Mode(mode)
- , Size(length)
- , Mapping() {
+mapped_file_region::mapped_file_region(int fd, mapmode mode, uint64_t length,
+ uint64_t offset, std::error_code &ec)
+ : Size(length), Mapping() {
// Make sure that the requested size fits within SIZE_T.
if (length > std::numeric_limits<size_t>::max()) {
ec = make_error_code(errc::invalid_argument);
return;
}
- ec = init(fd, closefd, offset);
+ ec = init(fd, offset, mode);
if (ec)
Mapping = nullptr;
}
@@ -495,11 +461,6 @@ mapped_file_region::~mapped_file_region(
::munmap(Mapping, Size);
}
-mapped_file_region::mapped_file_region(mapped_file_region &&other)
- : Mode(other.Mode), Size(other.Size), Mapping(other.Mapping) {
- other.Mapping = nullptr;
-}
-
uint64_t mapped_file_region::size() const {
assert(Mapping && "Mapping failed but used anyway!");
return Size;
@@ -507,7 +468,6 @@ uint64_t mapped_file_region::size() cons
char *mapped_file_region::data() const {
assert(Mapping && "Mapping failed but used anyway!");
- assert(Mode != readonly && "Cannot get non-const data for readonly mapping!");
return reinterpret_cast<char*>(Mapping);
}
Modified: llvm/trunk/lib/Support/Windows/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=224046&r1=224045&r2=224046&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Path.inc (original)
+++ llvm/trunk/lib/Support/Windows/Path.inc Thu Dec 11 14:12:55 2014
@@ -467,13 +467,12 @@ std::error_code setLastModificationAndAc
return std::error_code();
}
-std::error_code mapped_file_region::init(int FD, bool CloseFD, uint64_t Offset) {
+std::error_code mapped_file_region::init(int FD, uint64_t Offset,
+ mapmode Mode) {
FileDescriptor = FD;
// Make sure that the requested size fits within SIZE_T.
if (Size > std::numeric_limits<SIZE_T>::max()) {
if (FileDescriptor) {
- if (CloseFD)
- _close(FileDescriptor);
} else
::CloseHandle(FileHandle);
return make_error_code(errc::invalid_argument);
@@ -494,8 +493,6 @@ std::error_code mapped_file_region::init
if (FileMappingHandle == NULL) {
std::error_code ec = windows_error(GetLastError());
if (FileDescriptor) {
- if (CloseFD)
- _close(FileDescriptor);
} else
::CloseHandle(FileHandle);
return ec;
@@ -516,8 +513,6 @@ std::error_code mapped_file_region::init
std::error_code ec = windows_error(GetLastError());
::CloseHandle(FileMappingHandle);
if (FileDescriptor) {
- if (CloseFD)
- _close(FileDescriptor);
} else
::CloseHandle(FileHandle);
return ec;
@@ -531,8 +526,6 @@ std::error_code mapped_file_region::init
::UnmapViewOfFile(Mapping);
::CloseHandle(FileMappingHandle);
if (FileDescriptor) {
- if (CloseFD)
- _close(FileDescriptor);
} else
::CloseHandle(FileHandle);
return ec;
@@ -544,35 +537,23 @@ std::error_code mapped_file_region::init
// alive.
::CloseHandle(FileMappingHandle);
if (FileDescriptor) {
- if (CloseFD)
- _close(FileDescriptor); // Also closes FileHandle.
} else
::CloseHandle(FileHandle);
return std::error_code();
}
-mapped_file_region::mapped_file_region(int fd,
- bool closefd,
- mapmode mode,
- uint64_t length,
- uint64_t offset,
- std::error_code &ec)
- : Mode(mode)
- , Size(length)
- , Mapping()
- , FileDescriptor(fd)
- , FileHandle(INVALID_HANDLE_VALUE)
- , FileMappingHandle() {
+mapped_file_region::mapped_file_region(int fd, mapmode mode, uint64_t length,
+ uint64_t offset, std::error_code &ec)
+ : Size(length), Mapping(), FileDescriptor(fd),
+ FileHandle(INVALID_HANDLE_VALUE), FileMappingHandle() {
FileHandle = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
if (FileHandle == INVALID_HANDLE_VALUE) {
- if (closefd)
- _close(FileDescriptor);
FileDescriptor = 0;
ec = make_error_code(errc::bad_file_descriptor);
return;
}
- ec = init(FileDescriptor, closefd, offset);
+ ec = init(FileDescriptor, offset, mode);
if (ec) {
Mapping = FileMappingHandle = 0;
FileHandle = INVALID_HANDLE_VALUE;
@@ -585,25 +566,12 @@ mapped_file_region::~mapped_file_region(
::UnmapViewOfFile(Mapping);
}
-mapped_file_region::mapped_file_region(mapped_file_region &&other)
- : Mode(other.Mode)
- , Size(other.Size)
- , Mapping(other.Mapping)
- , FileDescriptor(other.FileDescriptor)
- , FileHandle(other.FileHandle)
- , FileMappingHandle(other.FileMappingHandle) {
- other.Mapping = other.FileMappingHandle = 0;
- other.FileHandle = INVALID_HANDLE_VALUE;
- other.FileDescriptor = 0;
-}
-
uint64_t mapped_file_region::size() const {
assert(Mapping && "Mapping failed but used anyway!");
return Size;
}
char *mapped_file_region::data() const {
- assert(Mode != readonly && "Cannot get non-const data for readonly mapping!");
assert(Mapping && "Mapping failed but used anyway!");
return reinterpret_cast<char*>(Mapping);
}
Modified: llvm/trunk/unittests/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=224046&r1=224045&r2=224046&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/Path.cpp (original)
+++ llvm/trunk/unittests/Support/Path.cpp Thu Dec 11 14:12:55 2014
@@ -649,11 +649,7 @@ TEST_F(FileSystemTest, FileMapping) {
StringRef Val("hello there");
{
fs::mapped_file_region mfr(FileDescriptor,
- true,
- fs::mapped_file_region::readwrite,
- 4096,
- 0,
- EC);
+ fs::mapped_file_region::readwrite, 4096, 0, EC);
ASSERT_NO_ERROR(EC);
std::copy(Val.begin(), Val.end(), mfr.data());
// Explicitly add a 0.
@@ -665,21 +661,16 @@ TEST_F(FileSystemTest, FileMapping) {
int FD;
EC = fs::openFileForRead(Twine(TempPath), FD);
ASSERT_NO_ERROR(EC);
- fs::mapped_file_region mfr(FD, false, fs::mapped_file_region::readonly, 0, 0,
- EC);
+ fs::mapped_file_region mfr(FD, fs::mapped_file_region::readonly, 0, 0, EC);
ASSERT_NO_ERROR(EC);
// Verify content
EXPECT_EQ(StringRef(mfr.const_data()), Val);
// Unmap temp file
- fs::mapped_file_region m(FD, false, fs::mapped_file_region::readonly, 0, 0,
- EC);
+ fs::mapped_file_region m(FD, fs::mapped_file_region::readonly, 0, 0, EC);
ASSERT_NO_ERROR(EC);
ASSERT_EQ(close(FD), 0);
- const char *Data = m.const_data();
- fs::mapped_file_region mfrrv(std::move(m));
- EXPECT_EQ(mfrrv.const_data(), Data);
}
TEST(Support, NormalizePath) {
More information about the llvm-commits
mailing list