From bfd3d062d6b4f4d88de838d639facda105043380 Mon Sep 17 00:00:00 2001 From: Tudor Bosman Date: Thu, 9 May 2013 14:16:35 -0700 Subject: [PATCH] Add missing FileUtil functions (p?readvFull, p?writevFull) , add test Summary: Testing incomplete reads / writes is hard, so I'm only testing the wrappers (template functions that take the underlying operation and retry it in case of incomplete operations). Note the cute hack of using variadic templates to use the same wrapper for both file pointer- and explicit-position flavors of the functions (the offset argument becomes optional). Test Plan: test added Reviewed By: lucian@fb.com FB internal diff: D806781 --- folly/FileUtil.cpp | 116 ++++------------ folly/FileUtil.h | 18 ++- folly/detail/FileUtilDetail.h | 112 ++++++++++++++++ folly/test/FileUtilTest.cpp | 245 ++++++++++++++++++++++++++++++++++ 4 files changed, 397 insertions(+), 94 deletions(-) create mode 100644 folly/detail/FileUtilDetail.h create mode 100644 folly/test/FileUtilTest.cpp diff --git a/folly/FileUtil.cpp b/folly/FileUtil.cpp index 8a07bf76..2e130a32 100644 --- a/folly/FileUtil.cpp +++ b/folly/FileUtil.cpp @@ -18,8 +18,12 @@ #include +#include "folly/detail/FileUtilDetail.h" + namespace folly { +using namespace fileutil_detail; + int closeNoInt(int fd) { int r = close(fd); // Ignore EINTR. On Linux, close() may only return EINTR after the file @@ -37,20 +41,6 @@ int closeNoInt(int fd) { return r; } -namespace { - -// Wrap call to f(args) in loop to retry on EINTR -template -ssize_t wrapNoInt(F f, Args... args) { - ssize_t r; - do { - r = f(args...); - } while (r == -1 && errno == EINTR); - return r; -} - -} // namespace - ssize_t readNoInt(int fd, void* buf, size_t count) { return wrapNoInt(read, fd, buf, count); } @@ -59,7 +49,7 @@ ssize_t preadNoInt(int fd, void* buf, size_t count, off_t offset) { return wrapNoInt(pread, fd, buf, count, offset); } -ssize_t readvNoInt(int fd, const struct iovec* iov, int count) { +ssize_t readvNoInt(int fd, const iovec* iov, int count) { return wrapNoInt(writev, fd, iov, count); } @@ -71,94 +61,40 @@ ssize_t pwriteNoInt(int fd, const void* buf, size_t count, off_t offset) { return wrapNoInt(pwrite, fd, buf, count, offset); } -ssize_t writevNoInt(int fd, const struct iovec* iov, int count) { +ssize_t writevNoInt(int fd, const iovec* iov, int count) { return wrapNoInt(writev, fd, iov, count); } ssize_t readFull(int fd, void* buf, size_t count) { - char* b = static_cast(buf); - ssize_t totalBytes = 0; - ssize_t r; - do { - r = read(fd, b, count); - if (r == -1) { - if (errno == EINTR) { - continue; - } - return r; - } - - totalBytes += r; - b += r; - count -= r; - } while (r != 0 && count); // 0 means EOF - - return totalBytes; + return wrapFull(read, fd, buf, count); } ssize_t preadFull(int fd, void* buf, size_t count, off_t offset) { - char* b = static_cast(buf); - ssize_t totalBytes = 0; - ssize_t r; - do { - r = pread(fd, b, count, offset); - if (r == -1) { - if (errno == EINTR) { - continue; - } - return r; - } - - totalBytes += r; - b += r; - offset += r; - count -= r; - } while (r != 0 && count); // 0 means EOF - - return totalBytes; + return wrapFull(pread, fd, buf, count, offset); } ssize_t writeFull(int fd, const void* buf, size_t count) { - const char* b = static_cast(buf); - ssize_t totalBytes = 0; - ssize_t r; - do { - r = write(fd, b, count); - if (r == -1) { - if (errno == EINTR) { - continue; - } - return r; - } - - totalBytes += r; - b += r; - count -= r; - } while (r != 0 && count); // 0 means EOF - - return totalBytes; + return wrapFull(write, fd, const_cast(buf), count); } ssize_t pwriteFull(int fd, const void* buf, size_t count, off_t offset) { - const char* b = static_cast(buf); - ssize_t totalBytes = 0; - ssize_t r; - do { - r = pwrite(fd, b, count, offset); - if (r == -1) { - if (errno == EINTR) { - continue; - } - return r; - } - - totalBytes += r; - b += r; - offset += r; - count -= r; - } while (r != 0 && count); // 0 means EOF - - return totalBytes; + return wrapFull(pwrite, fd, const_cast(buf), count, offset); +} + +ssize_t readvFull(int fd, iovec* iov, int count) { + return wrapvFull(readv, fd, iov, count); +} + +ssize_t preadvFull(int fd, iovec* iov, int count, off_t offset) { + return wrapvFull(preadv, fd, iov, count, offset); +} + +ssize_t writevFull(int fd, iovec* iov, int count) { + return wrapvFull(writev, fd, iov, count); +} + +ssize_t pwritevFull(int fd, iovec* iov, int count, off_t offset) { + return wrapvFull(pwritev, fd, iov, count, offset); } } // namespaces diff --git a/folly/FileUtil.h b/folly/FileUtil.h index 48e5e712..5845ba61 100644 --- a/folly/FileUtil.h +++ b/folly/FileUtil.h @@ -32,11 +32,11 @@ int closeNoInt(int fd); ssize_t readNoInt(int fd, void* buf, size_t n); ssize_t preadNoInt(int fd, void* buf, size_t n, off_t offset); -ssize_t readvNoInt(int fd, const struct iovec* iov, int count); +ssize_t readvNoInt(int fd, const iovec* iov, int count); ssize_t writeNoInt(int fd, const void* buf, size_t n); ssize_t pwriteNoInt(int fd, const void* buf, size_t n, off_t offset); -ssize_t writevNoInt(int fd, const struct iovec* iov, int count); +ssize_t writevNoInt(int fd, const iovec* iov, int count); /** * Wrapper around read() (and pread()) that, in addition to retrying on @@ -55,10 +55,15 @@ ssize_t writevNoInt(int fd, const struct iovec* iov, int count); * contiguous. You can no longer make this assumption if using readFull(). * You should probably use pread() when reading from the same file descriptor * from multiple threads simultaneously, anyway. + * + * Note that readvFull and preadvFull require iov to be non-const, unlike + * readv and preadv. The contents of iov after these functions return + * is unspecified. */ ssize_t readFull(int fd, void* buf, size_t n); ssize_t preadFull(int fd, void* buf, size_t n, off_t offset); -// TODO(tudorb): add readvFull if needed +ssize_t readvFull(int fd, iovec* iov, int count); +ssize_t preadvFull(int fd, iovec* iov, int count, off_t offset); /** * Similar to readFull and preadFull above, wrappers around write() and @@ -69,10 +74,15 @@ ssize_t preadFull(int fd, void* buf, size_t n, off_t offset); * a pipe), POSIX provides stronger guarantees, but not in the general case. * For example, Linux (even on a 64-bit platform) won't write more than 2GB in * one write() system call. + * + * Note that writevFull and pwritevFull require iov to be non-const, unlike + * writev and pwritev. The contents of iov after these functions return + * is unspecified. */ ssize_t writeFull(int fd, const void* buf, size_t n); ssize_t pwriteFull(int fd, const void* buf, size_t n, off_t offset); -// TODO(tudorb): add writevFull if needed +ssize_t writevFull(int fd, iovec* iov, int count); +ssize_t pwritevFull(int fd, iovec* iov, int count, off_t offset); } // namespaces diff --git a/folly/detail/FileUtilDetail.h b/folly/detail/FileUtilDetail.h new file mode 100644 index 00000000..108138b7 --- /dev/null +++ b/folly/detail/FileUtilDetail.h @@ -0,0 +1,112 @@ +/* + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FOLLY_DETAIL_FILEUTILDETAIL_H_ +#define FOLLY_DETAIL_FILEUTILDETAIL_H_ + +#include +#include + +#include + +/** + * Helper functions and templates for FileUtil.cpp. Declared here so + * they can be unittested. + */ +namespace folly { namespace fileutil_detail { + +// Wrap call to f(args) in loop to retry on EINTR +template +ssize_t wrapNoInt(F f, Args... args) { + ssize_t r; + do { + r = f(args...); + } while (r == -1 && errno == EINTR); + return r; +} + +inline void incr(ssize_t n) { } +inline void incr(ssize_t n, off_t& offset) { offset += n; } + +// Wrap call to read/pread/write/pwrite(fd, buf, count, offset?) to retry on +// incomplete reads / writes. The variadic argument magic is there to support +// an additional argument (offset) for pread / pwrite; see the incr() functions +// above which do nothing if the offset is not present and increment it if it +// is. +template +ssize_t wrapFull(F f, int fd, void* buf, size_t count, Offset... offset) { + char* b = static_cast(buf); + ssize_t totalBytes = 0; + ssize_t r; + do { + r = f(fd, b, count, offset...); + if (r == -1) { + if (errno == EINTR) { + continue; + } + return r; + } + + totalBytes += r; + b += r; + count -= r; + incr(r, offset...); + } while (r != 0 && count); // 0 means EOF + + return totalBytes; +} + +// Wrap call to readv/preadv/writev/pwritev(fd, iov, count, offset?) to +// retry on incomplete reads / writes. +template +ssize_t wrapvFull(F f, int fd, iovec* iov, int count, Offset... offset) { + ssize_t totalBytes = 0; + ssize_t r; + do { + r = f(fd, iov, count, offset...); + if (r == -1) { + if (errno == EINTR) { + continue; + } + return r; + } + + if (r == 0) { + break; // EOF + } + + totalBytes += r; + incr(r, offset...); + while (r != 0 && count != 0) { + if (r >= iov->iov_len) { + r -= iov->iov_len; + ++iov; + --count; + } else { + iov->iov_base = static_cast(iov->iov_base) + r; + iov->iov_len -= r; + r = 0; + } + } + } while (count); + + return totalBytes; +} + +}} // namespaces + +#endif /* FOLLY_DETAIL_FILEUTILDETAIL_H_ */ + diff --git a/folly/test/FileUtilTest.cpp b/folly/test/FileUtilTest.cpp new file mode 100644 index 00000000..8b50b5cf --- /dev/null +++ b/folly/test/FileUtilTest.cpp @@ -0,0 +1,245 @@ +/* + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "folly/FileUtil.h" +#include "folly/detail/FileUtilDetail.h" + +#include + +#include +#include +#include + +#include "folly/Benchmark.h" +#include "folly/Range.h" +#include "folly/String.h" + +namespace folly { namespace test { + +using namespace fileutil_detail; + +namespace { + +class Reader { + public: + Reader(off_t offset, StringPiece data, std::deque spec); + + // write-like + ssize_t operator()(int fd, void* buf, size_t count); + + // pwrite-like + ssize_t operator()(int fd, void* buf, size_t count, off_t offset); + + // writev-like + ssize_t operator()(int fd, const iovec* iov, int count); + + // pwritev-like + ssize_t operator()(int fd, const iovec* iov, int count, off_t offset); + + const std::deque spec() const { return spec_; } + + private: + ssize_t nextSize(); + + off_t offset_; + StringPiece data_; + std::deque spec_; +}; + +Reader::Reader(off_t offset, StringPiece data, std::deque spec) + : offset_(offset), + data_(data), + spec_(std::move(spec)) { +} + +ssize_t Reader::nextSize() { + if (spec_.empty()) { + throw std::runtime_error("spec empty"); + } + ssize_t n = spec_.front(); + spec_.pop_front(); + if (n <= 0) { + if (n == -1) { + errno = EIO; + } + spec_.clear(); // so we fail if called again + } else { + offset_ += n; + } + return n; +} + +ssize_t Reader::operator()(int fd, void* buf, size_t count) { + ssize_t n = nextSize(); + if (n <= 0) { + return n; + } + if (n > count) { + throw std::runtime_error("requested count too small"); + } + memcpy(buf, data_.data(), n); + data_.advance(n); + return n; +} + +ssize_t Reader::operator()(int fd, void* buf, size_t count, off_t offset) { + EXPECT_EQ(offset_, offset); + return operator()(fd, buf, count); +} + +ssize_t Reader::operator()(int fd, const iovec* iov, int count) { + ssize_t n = nextSize(); + if (n <= 0) { + return n; + } + ssize_t remaining = n; + for (; count != 0 && remaining != 0; ++iov, --count) { + ssize_t len = std::min(remaining, ssize_t(iov->iov_len)); + memcpy(iov->iov_base, data_.data(), len); + data_.advance(len); + remaining -= len; + } + if (remaining != 0) { + throw std::runtime_error("requested total size too small"); + } + return n; +} + +ssize_t Reader::operator()(int fd, const iovec* iov, int count, off_t offset) { + EXPECT_EQ(offset_, offset); + return operator()(fd, iov, count); +} + +} // namespace + +class FileUtilTest : public ::testing::Test { + protected: + FileUtilTest(); + + Reader reader(std::deque spec); + + std::string in_; + std::vector> readers_; +}; + +FileUtilTest::FileUtilTest() + : in_("1234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") { + CHECK_EQ(62, in_.size()); + + readers_.emplace_back(0, reader({0})); + readers_.emplace_back(62, reader({62})); + readers_.emplace_back(62, reader({62, -1})); // error after end (not called) + readers_.emplace_back(61, reader({61, 0})); + readers_.emplace_back(-1, reader({61, -1})); // error before end + readers_.emplace_back(62, reader({31, 31})); + readers_.emplace_back(62, reader({1, 10, 20, 10, 1, 20})); + readers_.emplace_back(61, reader({1, 10, 20, 10, 20, 0})); + readers_.emplace_back(41, reader({1, 10, 20, 10, 0})); + readers_.emplace_back(-1, reader({1, 10, 20, 10, 20, -1})); +} + +Reader FileUtilTest::reader(std::deque spec) { + return Reader(42, in_, std::move(spec)); +} + +TEST_F(FileUtilTest, read) { + for (auto& p : readers_) { + std::string out(in_.size(), '\0'); + EXPECT_EQ(p.first, wrapFull(p.second, 0, &out[0], out.size())); + if (p.first != -1) { + EXPECT_EQ(in_.substr(0, p.first), out.substr(0, p.first)); + } + } +} + +TEST_F(FileUtilTest, pread) { + for (auto& p : readers_) { + std::string out(in_.size(), '\0'); + EXPECT_EQ(p.first, wrapFull(p.second, 0, &out[0], out.size(), off_t(42))); + if (p.first != -1) { + EXPECT_EQ(in_.substr(0, p.first), out.substr(0, p.first)); + } + } +} + +class IovecBuffers { + public: + explicit IovecBuffers(std::initializer_list sizes); + + std::vector iov() const { return iov_; } // yes, make a copy + std::string join() const { return folly::join("", buffers_); } + size_t size() const; + + private: + std::vector buffers_; + std::vector iov_; +}; + +IovecBuffers::IovecBuffers(std::initializer_list sizes) { + iov_.reserve(sizes.size()); + for (auto& s : sizes) { + buffers_.push_back(std::string(s, '\0')); + iovec iov; + iov.iov_base = &(buffers_.back()[0]); + iov.iov_len = s; + iov_.push_back(iov); + } +} + +size_t IovecBuffers::size() const { + size_t s = 0; + for (auto& b : buffers_) { + s += b.size(); + } + return s; +} + +TEST_F(FileUtilTest, readv) { + for (auto& p : readers_) { + IovecBuffers buf({12, 19, 31}); + ASSERT_EQ(62, buf.size()); + + auto iov = buf.iov(); + EXPECT_EQ(p.first, wrapvFull(p.second, 0, iov.data(), iov.size())); + if (p.first != -1) { + EXPECT_EQ(in_.substr(0, p.first), buf.join().substr(0, p.first)); + } + } +} + +TEST_F(FileUtilTest, preadv) { + for (auto& p : readers_) { + IovecBuffers buf({12, 19, 31}); + ASSERT_EQ(62, buf.size()); + + auto iov = buf.iov(); + EXPECT_EQ(p.first, + wrapvFull(p.second, 0, iov.data(), iov.size(), off_t(42))); + if (p.first != -1) { + EXPECT_EQ(in_.substr(0, p.first), buf.join().substr(0, p.first)); + } + } +} + + +}} // namespaces + +int main(int argc, char *argv[]) { + testing::InitGoogleTest(&argc, argv); + google::ParseCommandLineFlags(&argc, &argv, true); + return RUN_ALL_TESTS(); +} + -- 2.34.1