summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHenning Baldersheim <balder@yahoo-inc.com>2023-08-15 13:39:28 +0000
committerHenning Baldersheim <balder@yahoo-inc.com>2023-08-15 14:04:52 +0000
commite945442941c923b09ac67b2fcd63a4f115ebd638 (patch)
treedec9458fe7ad85f957c09bf57dc6ed0d5f84f154
parente72ccbe7d79e47235414909720097c648476eab5 (diff)
GC and clean up more unused code
-rw-r--r--document/src/tests/serialization/vespadocumentserializer_test.cpp2
-rw-r--r--vespalib/src/tests/io/fileutil/fileutiltest.cpp33
-rw-r--r--vespalib/src/vespa/vespalib/io/fileutil.cpp152
-rw-r--r--vespalib/src/vespa/vespalib/io/fileutil.h54
4 files changed, 46 insertions, 195 deletions
diff --git a/document/src/tests/serialization/vespadocumentserializer_test.cpp b/document/src/tests/serialization/vespadocumentserializer_test.cpp
index 1839005d720..03878f43e4b 100644
--- a/document/src/tests/serialization/vespadocumentserializer_test.cpp
+++ b/document/src/tests/serialization/vespadocumentserializer_test.cpp
@@ -686,7 +686,7 @@ void deserializeAndCheck(const string &file_name, FieldValueT &value,
const string &field_name) {
File file(file_name);
file.open(File::READONLY);
- vector<char> content(file.stat()._size);
+ vector<char> content(file.getFileSize());
size_t r = file.read(&content[0], content.size(), 0);
ASSERT_EQUAL(content.size(), r);
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();
};
/**