Add missing FileUtil functions (p?readvFull, p?writevFull) , add test
authorTudor Bosman <tudorb@fb.com>
Thu, 9 May 2013 21:16:35 +0000 (14:16 -0700)
committerSara Golemon <sgolemon@fb.com>
Mon, 20 May 2013 18:01:27 +0000 (11:01 -0700)
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
folly/FileUtil.h
folly/detail/FileUtilDetail.h [new file with mode: 0644]
folly/test/FileUtilTest.cpp [new file with mode: 0644]

index 8a07bf768e262d17c66e84f7c432c3bf411d9666..2e130a32ad4cb311f8f06421b4bdb01727fe5bcf 100644 (file)
 
 #include <cerrno>
 
+#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<typename F, typename... Args>
-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<char*>(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<char*>(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<const char*>(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<void*>(buf), count);
 }
 
 ssize_t pwriteFull(int fd, const void* buf, size_t count, off_t offset) {
-  const char* b = static_cast<const char*>(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<void*>(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
index 48e5e7129a5e8e800671b99e3b6b480df0d7562c..5845ba61d385db4b4c6ce97ef0084d135a7b912a 100644 (file)
@@ -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 (file)
index 0000000..108138b
--- /dev/null
@@ -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 <cerrno>
+#include <unistd.h>
+
+#include <sys/uio.h>
+
+/**
+ * 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<class F, class... Args>
+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 <class F, class... Offset>
+ssize_t wrapFull(F f, int fd, void* buf, size_t count, Offset... offset) {
+  char* b = static_cast<char*>(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 <class F, class... Offset>
+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<char*>(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 (file)
index 0000000..8b50b5c
--- /dev/null
@@ -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 <deque>
+
+#include <glog/logging.h>
+#include <gflags/gflags.h>
+#include <gtest/gtest.h>
+
+#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<ssize_t> 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<ssize_t> spec() const { return spec_; }
+
+ private:
+  ssize_t nextSize();
+
+  off_t offset_;
+  StringPiece data_;
+  std::deque<ssize_t> spec_;
+};
+
+Reader::Reader(off_t offset, StringPiece data, std::deque<ssize_t> 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<ssize_t> spec);
+
+  std::string in_;
+  std::vector<std::pair<size_t, Reader>> 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<ssize_t> 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<size_t> sizes);
+
+  std::vector<iovec> iov() const { return iov_; }  // yes, make a copy
+  std::string join() const { return folly::join("", buffers_); }
+  size_t size() const;
+
+ private:
+  std::vector<std::string> buffers_;
+  std::vector<iovec> iov_;
+};
+
+IovecBuffers::IovecBuffers(std::initializer_list<size_t> 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();
+}
+