diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-08-15 13:39:28 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2023-08-15 14:04:52 +0000 |
commit | e945442941c923b09ac67b2fcd63a4f115ebd638 (patch) | |
tree | dec9458fe7ad85f957c09bf57dc6ed0d5f84f154 /vespalib/src | |
parent | e72ccbe7d79e47235414909720097c648476eab5 (diff) |
GC and clean up more unused code
Diffstat (limited to 'vespalib/src')
-rw-r--r-- | vespalib/src/tests/io/fileutil/fileutiltest.cpp | 33 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/io/fileutil.cpp | 152 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/io/fileutil.h | 54 |
3 files changed, 45 insertions, 194 deletions
diff --git a/vespalib/src/tests/io/fileutil/fileutiltest.cpp b/vespalib/src/tests/io/fileutil/fileutiltest.cpp index 5d1d1a14475..0289bb376ad 100644 --- a/vespalib/src/tests/io/fileutil/fileutiltest.cpp +++ b/vespalib/src/tests/io/fileutil/fileutiltest.cpp @@ -126,16 +126,6 @@ TEST("require that vespalib::File::open works") //std::cerr << e.what() << "\n"; EXPECT_EQUAL(IoException::ILLEGAL_PATH, e.getType()); } - // Test opening already open file - { - std::unique_ptr<File> f(new File("myfile")); - f->open(File::CREATE, false); - f->closeFileWhenDestructed(false); - File f2(f->getFileDescriptor(), "myfile"); - f.reset(); - ASSERT_TRUE(f2.isOpen()); - f2.write(" ", 1, 0); - } // Test reopening file in same object { File f("myfile"); @@ -161,29 +151,6 @@ TEST("require that vespalib::File::isOpen works") ASSERT_TRUE(!f.isOpen()); } -TEST("require that vespalib::File::stat works") -{ - std::filesystem::remove(std::filesystem::path("myfile")); - std::filesystem::remove_all(std::filesystem::path("mydir")); - EXPECT_EQUAL(false, fileExists("myfile")); - EXPECT_EQUAL(false, fileExists("mydir")); - std::filesystem::create_directory(std::filesystem::path("mydir")); - File f("myfile"); - f.open(File::CREATE, false); - f.write("foobar", 6, 0); - - FileInfo info = f.stat(); - EXPECT_EQUAL(6, info._size); - EXPECT_EQUAL(true, info._plainfile); - EXPECT_EQUAL(false, info._directory); - - EXPECT_EQUAL(6, f.getFileSize()); - f.close(); - - EXPECT_EQUAL(true, fileExists("myfile")); - EXPECT_EQUAL(true, fileExists("mydir")); -} - TEST("require that vespalib::File::resize works") { std::filesystem::remove(std::filesystem::path("myfile")); diff --git a/vespalib/src/vespa/vespalib/io/fileutil.cpp b/vespalib/src/vespa/vespalib/io/fileutil.cpp index 0a70b343b13..28455d3dd42 100644 --- a/vespalib/src/vespa/vespalib/io/fileutil.cpp +++ b/vespalib/src/vespa/vespalib/io/fileutil.cpp @@ -5,7 +5,6 @@ #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/vespalib/util/size_literals.h> #include <vespa/vespalib/util/stringfmt.h> -#include <ostream> #include <cassert> #include <filesystem> #include <dirent.h> @@ -16,6 +15,8 @@ #include <vespa/log/log.h> LOG_SETUP(".vespalib.io.fileutil"); +namespace fs = std::filesystem; + namespace vespalib { namespace { @@ -27,7 +28,6 @@ processStat(struct stat& filestats, bool result, stringref path) { resval = std::make_unique<FileInfo>(); resval->_plainfile = S_ISREG(filestats.st_mode); resval->_directory = S_ISDIR(filestats.st_mode); - resval->_symlink = S_ISLNK(filestats.st_mode); resval->_size = filestats.st_size; } else if (errno != ENOENT) { asciistream ost; @@ -52,79 +52,15 @@ safeStrerror(int errnum) } -bool -FileInfo::operator==(const FileInfo& fi) const -{ - return (_size == fi._size && _plainfile == fi._plainfile - && _directory == fi._directory); -} - -std::ostream& -operator<<(std::ostream& out, const FileInfo& info) -{ - out << "FileInfo(size: " << info._size; - if (info._plainfile) out << ", plain file"; - if (info._directory) out << ", directory"; - out << ")"; - return out; -} - File::File(stringref filename) : _fd(-1), _flags(0), - _filename(filename), - _close(true), - _fileReads(0), - _fileWrites(0) -{ -} - -File::File(int fileDescriptor, stringref filename) - : _fd(fileDescriptor), - _flags(0), - _filename(filename), - _close(true), - _fileReads(0), - _fileWrites(0) -{ -} + _filename(filename) +{ } File::~File() { - if (_close && _fd != -1) close(); -} - -File::File(File& f) - : _fd(f._fd), - _flags(f._flags), - _filename(f._filename), - _close(f._close), - _fileReads(f._fileReads), - _fileWrites(f._fileWrites) -{ - f._fd = -1; - f._flags = 0; - f._close = true; - f._fileReads = 0; - f._fileWrites = 0; -} - -File& -File::operator=(File& f) -{ - if (_close && _fd != -1) close(); - _fd = f._fd; - _flags = f._flags; - _filename = f._filename; - _close = f._close; - _fileReads = f._fileReads; - _fileWrites = f._fileWrites; - f._fd = -1; - f._flags = 0; - f._close = true; - f._fileReads = 0; - f._fileWrites = 0; - return *this; + if (_fd != -1) close(); } namespace { @@ -137,9 +73,8 @@ int openAndCreateDirsIfMissing(const string & filename, int flags, bool createDi auto pos = filename.rfind('/'); if (pos != string::npos) { string path(filename.substr(0, pos)); - std::filesystem::create_directories(std::filesystem::path(path)); - LOG(spam, "open(%s, %d): Retrying open after creating parent " - "directories.", filename.c_str(), flags); + fs::create_directories(fs::path(path)); + LOG(spam, "open(%s, %d): Retrying open after creating parent directories.", filename.c_str(), flags); fd = ::open(filename.c_str(), flags, 0644); } } @@ -151,19 +86,13 @@ void File::open(int flags, bool autoCreateDirectories) { if ((flags & File::READONLY) != 0) { if ((flags & File::CREATE) != 0) { - throw IllegalArgumentException( - "Cannot use READONLY and CREATE options at the same time", - VESPA_STRLOC); + throw IllegalArgumentException("Cannot use READONLY and CREATE options at the same time", VESPA_STRLOC); } if ((flags & File::TRUNC) != 0) { - throw IllegalArgumentException( - "Cannot use READONLY and TRUNC options at the same time", - VESPA_STRLOC); + throw IllegalArgumentException("Cannot use READONLY and TRUNC options at the same time", VESPA_STRLOC); } if (autoCreateDirectories) { - throw IllegalArgumentException( - "No point in auto-creating directories on read only access", - VESPA_STRLOC); + throw IllegalArgumentException("No point in auto-creating directories on read only access", VESPA_STRLOC); } } int openflags = ((flags & File::READONLY) != 0 ? O_RDONLY : O_RDWR) @@ -177,30 +106,21 @@ File::open(int flags, bool autoCreateDirectories) { if (fd < 0 && ((flags & File::DIRECTIO) != 0)) { openflags = (openflags ^ O_DIRECT); flags = (flags ^ DIRECTIO); - LOG(debug, "open(%s, %d): Retrying without direct IO due to failure " - "opening with errno(%d): %s", + LOG(debug, "open(%s, %d): Retrying without direct IO due to failure opening with errno(%d): %s", _filename.c_str(), flags, errno, safeStrerror(errno).c_str()); fd = openAndCreateDirsIfMissing(_filename, openflags, autoCreateDirectories); } #endif if (fd < 0) { asciistream ost; - ost << "open(" << _filename << ", 0x" - << hex << flags << dec << "): Failed, errno(" << errno - << "): " << safeStrerror(errno); + ost << "open(" << _filename << ", 0x" << hex << flags << dec + << "): Failed, errno(" << errno << "): " << safeStrerror(errno); throw IoException(ost.str(), IoException::getErrorType(errno), VESPA_STRLOC); } _flags = flags; - if (_close && _fd != -1) close(); + if (_fd != -1) close(); _fd = fd; - LOG(debug, "open(%s, %d). File opened with file descriptor %d.", - _filename.c_str(), flags, fd); -} - -void -File::closeFileWhenDestructed(bool closeOnDestruct) -{ - _close = closeOnDestruct; + LOG(debug, "open(%s, %d). File opened with file descriptor %d.", _filename.c_str(), flags, fd); } FileInfo @@ -212,13 +132,11 @@ File::stat() const result = processStat(filestats, fstat(_fd, &filestats) == 0, _filename); assert(result.get()); // The file must exist in a file instance } else { - result = processStat(filestats, - ::stat(_filename.c_str(), &filestats) == 0, - _filename); + result = processStat(filestats, ::stat(_filename.c_str(), &filestats) == 0, _filename); // If the file does not exist yet, act like it does. It will // probably be created when opened. if (result.get() == 0) { - result.reset(new FileInfo()); + result = std::make_unique<FileInfo>(); result->_size = 0; result->_directory = false; result->_plainfile = true; @@ -232,12 +150,10 @@ File::resize(off_t size) { if (ftruncate(_fd, size) != 0) { asciistream ost; - ost << "resize(" << _filename << ", " << size << "): Failed, errno(" - << errno << "): " << safeStrerror(errno); + ost << "resize(" << _filename << ", " << size << "): Failed, errno(" << errno << "): " << safeStrerror(errno); throw IoException(ost.str(), IoException::getErrorType(errno), VESPA_STRLOC); } - LOG(debug, "resize(%s): Resized to %" PRIu64 " bytes.", - _filename.c_str(), size); + LOG(debug, "resize(%s): Resized to %" PRIu64 " bytes.", _filename.c_str(), size); } void @@ -269,10 +185,8 @@ File::verifyDirectIO(uint64_t buf, size_t bufsize, off_t offset) const off_t File::write(const void *buf, size_t bufsize, off_t offset) { - ++_fileWrites; size_t left = bufsize; - LOG(debug, "write(%s): Writing %zu bytes at offset %" PRIu64 ".", - _filename.c_str(), bufsize, offset); + LOG(debug, "write(%s): Writing %zu bytes at offset %" PRIu64 ".", _filename.c_str(), bufsize, offset); if (_flags & DIRECTIO) { verifyDirectIO((uint64_t)buf, bufsize, offset); @@ -281,20 +195,17 @@ File::write(const void *buf, size_t bufsize, off_t offset) while (left > 0) { ssize_t written = ::pwrite(_fd, buf, left, offset); if (written > 0) { - LOG(spam, "write(%s): Wrote %zd bytes at offset %" PRIu64 ".", - _filename.c_str(), written, offset); + LOG(spam, "write(%s): Wrote %zd bytes at offset %" PRIu64 ".", _filename.c_str(), written, offset); left -= written; buf = ((const char*) buf) + written; offset += written; } else if (written == 0) { - LOG(spam, "write(%s): Wrote %zd bytes at offset %" PRIu64 ".", - _filename.c_str(), written, offset); + LOG(spam, "write(%s): Wrote %zd bytes at offset %" PRIu64 ".", _filename.c_str(), written, offset); assert(false); // Can this happen? } else if (errno != EINTR && errno != EAGAIN) { asciistream ost; - ost << "write(" << _fd << ", " << buf - << ", " << left << ", " << offset << "), Failed, errno(" - << errno << "): " << safeStrerror(errno); + ost << "write(" << _fd << ", " << buf << ", " << left << ", " << offset + << "), Failed, errno(" << errno << "): " << safeStrerror(errno); throw IoException(ost.str(), IoException::getErrorType(errno), VESPA_STRLOC); } } @@ -304,10 +215,8 @@ File::write(const void *buf, size_t bufsize, off_t offset) size_t File::read(void *buf, size_t bufsize, off_t offset) const { - ++_fileReads; size_t remaining = bufsize; - LOG(debug, "read(%s): Reading %zu bytes from offset %" PRIu64 ".", - _filename.c_str(), bufsize, offset); + LOG(debug, "read(%s): Reading %zu bytes from offset %" PRIu64 ".", _filename.c_str(), bufsize, offset); if (_flags & DIRECTIO) { verifyDirectIO((uint64_t)buf, bufsize, offset); @@ -316,8 +225,7 @@ File::read(void *buf, size_t bufsize, off_t offset) const while (remaining > 0) { ssize_t bytesread = ::pread(_fd, buf, remaining, offset); if (bytesread > 0) { - LOG(spam, "read(%s): Read %zd bytes from offset %" PRIu64 ".", - _filename.c_str(), bytesread, offset); + LOG(spam, "read(%s): Read %zd bytes from offset %" PRIu64 ".", _filename.c_str(), bytesread, offset); remaining -= bytesread; buf = ((char*) buf) + bytesread; offset += bytesread; @@ -333,8 +241,7 @@ File::read(void *buf, size_t bufsize, off_t offset) const } else if (errno != EINTR && errno != EAGAIN) { asciistream ost; ost << "read(" << _fd << ", " << buf << ", " << remaining << ", " - << offset << "): Failed, errno(" << errno << "): " - << safeStrerror(errno); + << offset << "): Failed, errno(" << errno << "): " << safeStrerror(errno); throw IoException(ost.str(), IoException::getErrorType(errno), VESPA_STRLOC); } } @@ -419,7 +326,7 @@ bool File::unlink() { close(); - return std::filesystem::remove(std::filesystem::path(_filename)); + return fs::remove(fs::path(_filename)); } DirectoryList @@ -487,8 +394,7 @@ getOpenErrorString(const int osError, stringref filename) { asciistream os; string dirName(dirname(filename)); - os << "error=" << osError << "(\"" << - getErrorString(osError) << "\") fileStat"; + os << "error=" << osError << "(\"" << getErrorString(osError) << "\") fileStat"; addStat(os, filename); os << " dirStat"; addStat(os, dirName); diff --git a/vespalib/src/vespa/vespalib/io/fileutil.h b/vespalib/src/vespa/vespalib/io/fileutil.h index 29e9bd66918..1d66c026ecf 100644 --- a/vespalib/src/vespa/vespalib/io/fileutil.h +++ b/vespalib/src/vespa/vespalib/io/fileutil.h @@ -43,14 +43,10 @@ struct FileInfo { bool _plainfile; bool _directory; - bool _symlink; off_t _size; - bool operator==(const FileInfo&) const; }; -std::ostream& operator<<(std::ostream&, const FileInfo&); - /** * @brief A File instance is used to access a single open file. * @@ -64,19 +60,18 @@ private: int _fd; int _flags; string _filename; - bool _close; - mutable int _fileReads; // Tracks number of file reads done on this file - mutable int _fileWrites; // Tracks number of file writes done in this file - /** * Verify that direct I/O alignment preconditions hold. Triggers assertion * failure on violations. */ void verifyDirectIO(uint64_t buf, size_t bufsize, off_t offset) const; - - /** Copying a file instance, moves any open file descriptor. */ - File(File& f); - File& operator=(File& f); + void sync(); + /** + * Get information about the current file. If file is opened, file descriptor + * will be used for stat. If file is not open, and the file does not exist + * yet, you will get fileinfo describing an empty file. + */ + FileInfo stat() const; public: using UP = std::unique_ptr<File>; @@ -88,40 +83,24 @@ public: /** Create a file instance, without opening the file. */ File(stringref filename); - /** Create a file instance of an already open file. */ - File(int fileDescriptor, stringref filename); - /** Closes the file if not instructed to do otherwise. */ - virtual ~File(); + ~File(); const string& getFilename() const { return _filename; } - virtual void open(int flags, bool autoCreateDirectories = false); + void open(int flags, bool autoCreateDirectories = false); bool isOpen() const { return (_fd != -1); } bool isOpenWithDirectIO() const { return ((_flags & DIRECTIO) != 0); } - /** - * Whether or not file should be closed when this instance is destructed. - * By default it will be closed. - */ - void closeFileWhenDestructed(bool close); - - virtual int getFileDescriptor() const { return _fd; } - - /** - * Get information about the current file. If file is opened, file descriptor - * will be used for stat. If file is not open, and the file does not exist - * yet, you will get fileinfo describing an empty file. - */ - virtual FileInfo stat() const; + int getFileDescriptor() const { return _fd; } /** * Get the filesize of a file, specified by a file descriptor. * * @throw IoException If we failed to stat the file. */ - virtual off_t getFileSize() const { return stat()._size; } + off_t getFileSize() const { return stat()._size; } /** * Resize the currently open file to a given size, @@ -131,7 +110,7 @@ public: * @param size new size of file * @throw IoException If we failed to resize the file. */ - virtual void resize(off_t size); + void resize(off_t size); /** * Writes data to file. @@ -145,7 +124,7 @@ public: * @throw IoException If we failed to write to the file. * @return Always return bufsize. */ - virtual off_t write(const void *buf, size_t bufsize, off_t offset); + off_t write(const void *buf, size_t bufsize, off_t offset); /** * Read characters from a file. @@ -160,7 +139,7 @@ public: * @return The number of bytes actually read. If less than * bufsize, this indicates that EOF was reached. */ - virtual size_t read(void *buf, size_t bufsize, off_t offset) const; + size_t read(void *buf, size_t bufsize, off_t offset) const; /** * Read the file into a string. @@ -193,9 +172,8 @@ public: */ static void sync(stringref path); - virtual void sync(); - virtual bool close(); - virtual bool unlink(); + bool close(); + bool unlink(); }; /** |