add Cursor::peekBytes()

Summary:
The existing Cursor::peek() method pre-dates the existence of folly::ByteRange,
and so it returns a std::pair containing a pointer and length.  This is usually
more awkward to use than ByteRange.

This adds a peekBytes() method that returns a ByteRange object, and updates all
users of peek() in folly to use peekBytes() instead.  Eventually I think we
should add a FOLLY_DEPRECATED attribute to peek().  I will wait to add this tag
until after converting a few other projects to use peekBytes(), though.

Reviewed By: alandau

Differential Revision: D3337302

fbshipit-source-id: 14220a059d915bf5adc66b8b283f7228b796a4dc

diff --git a/folly/Subprocess.cpp b/folly/Subprocess.cpp
index 769cbebb5f51c184106a24836fff35f3e263c37c..8190c47063b64737b1a277ba579caab20b5cf3ae 100644 (file)
--- a/folly/Subprocess.cpp
+++ b/folly/Subprocess.cpp
@@ -606,21 +606,23 @@ pid_t Subprocess::pid() const {
 
 namespace {
 
-std::pair<const uint8_t*, size_t> queueFront(const IOBufQueue& queue) {
+ByteRange queueFront(const IOBufQueue& queue) {
   auto* p = queue.front();
-  if (!p) return std::make_pair(nullptr, 0);
-  return io::Cursor(p).peek();
+  if (!p) {
+    return ByteRange{};
+  }
+  return io::Cursor(p).peekBytes();
 }
 
 // fd write
 bool handleWrite(int fd, IOBufQueue& queue) {
   for (;;) {
-    auto p = queueFront(queue);
-    if (p.second == 0) {
+    auto b = queueFront(queue);
+    if (b.empty()) {
       return true;  // EOF
     }
 
-    ssize_t n = writeNoInt(fd, p.first, p.second);
+    ssize_t n = writeNoInt(fd, b.data(), b.size());
     if (n == -1 && errno == EAGAIN) {
       return false;
     }

diff --git a/folly/experimental/bser/Load.cpp b/folly/experimental/bser/Load.cpp
index 4d311f9f99ff45706808efe10c2ec0f34b67be47..2bd473b3354ec4cbad6ce1ec49535da47c4c6c92 100644 (file)
--- a/folly/experimental/bser/Load.cpp
+++ b/folly/experimental/bser/Load.cpp
@@ -116,8 +116,8 @@ static dynamic decodeTemplate(Cursor& curs) {
     dynamic obj = dynamic::object;
 
     for (auto& name : names) {
-      auto pair = curs.peek();
-      if ((BserType)pair.first[0] == BserType::Skip) {
+      auto bytes = curs.peekBytes();
+      if ((BserType)bytes.at(0) == BserType::Skip) {
         obj[name.getString()] = nullptr;
         curs.skipAtMost(1);
         continue;
@@ -176,7 +176,7 @@ static size_t decodeHeader(Cursor& curs) {
     throw std::runtime_error("invalid BSER magic header");
   }
 
-  auto enc = (BserType)curs.peek().first[0];
+  auto enc = (BserType)curs.peekBytes().at(0);
   size_t int_size;
   switch (enc) {
     case BserType::Int8:

diff --git a/folly/io/Compression.cpp b/folly/io/Compression.cpp
index 21190f5cbc7fcf20b10bb063684af45b1c3e9fca..356699cf49f2487635d6e21fc97049469d92c934 100644 (file)
--- a/folly/io/Compression.cpp
+++ b/folly/io/Compression.cpp
@@ -303,12 +303,13 @@ std::unique_ptr<IOBuf> LZ4Codec::doUncompress(
     }
   }
 
-  auto p = cursor.peek();
+  auto sp = StringPiece{cursor.peekBytes()};
   auto out = IOBuf::create(actualUncompressedLength);
-  int n = LZ4_decompress_safe(reinterpret_cast<const char*>(p.first),
-                              reinterpret_cast<char*>(out->writableTail()),
-                              p.second,
-                              actualUncompressedLength);
+  int n = LZ4_decompress_safe(
+      sp.data(),
+      reinterpret_cast<char*>(out->writableTail()),
+      sp.size(),
+      actualUncompressedLength);
 
   if (n < 0 || uint64_t(n) != actualUncompressedLength) {
     throw std::runtime_error(to<std::string>(
@@ -350,9 +351,9 @@ size_t IOBufSnappySource::Available() const {
 }
 
 const char* IOBufSnappySource::Peek(size_t* len) {
-  auto p = cursor_.peek();
-  *len = p.second;
-  return reinterpret_cast<const char*>(p.first);
+  auto sp = StringPiece{cursor_.peekBytes()};
+  *len = sp.size();
+  return sp.data();
 }
 
 void IOBufSnappySource::Skip(size_t n) {
@@ -908,10 +909,10 @@ std::unique_ptr<IOBuf> LZMA2Codec::doUncompress(const IOBuf* data,
        defaultBufferLength));
 
   bool streamEnd = false;
-  auto buf = cursor.peek();
-  while (buf.second != 0) {
-    stream.next_in = const_cast<uint8_t*>(buf.first);
-    stream.avail_in = buf.second;
+  auto buf = cursor.peekBytes();
+  while (!buf.empty()) {
+    stream.next_in = const_cast<uint8_t*>(buf.data());
+    stream.avail_in = buf.size();
 
     while (stream.avail_in != 0) {
       if (streamEnd) {
@@ -922,8 +923,8 @@ std::unique_ptr<IOBuf> LZMA2Codec::doUncompress(const IOBuf* data,
       streamEnd = doInflate(&stream, out.get(), defaultBufferLength);
     }
 
-    cursor.skip(buf.second);
-    buf = cursor.peek();
+    cursor.skip(buf.size());
+    buf = cursor.peekBytes();
   }
 
   while (!streamEnd) {

diff --git a/folly/io/Cursor.h b/folly/io/Cursor.h
index a13092394a557781a9e420bc152ff3c592a64fd1..5e1ce527b5c0ed28548471280460df7a719af436 100644 (file)
--- a/folly/io/Cursor.h
+++ b/folly/io/Cursor.h
@@ -86,10 +86,10 @@ class CursorBase {
   /**
    * Return the remaining space available in the current IOBuf.
    *
-   * May return 0 if the cursor is at the end of an IOBuf.  Use peek() instead
-   * if you want to avoid this.  peek() will advance to the next non-empty
-   * IOBuf (up to the end of the chain) if the cursor is currently pointing at
-   * the end of a buffer.
+   * May return 0 if the cursor is at the end of an IOBuf.  Use peekBytes()
+   * instead if you want to avoid this.  peekBytes() will advance to the next
+   * non-empty IOBuf (up to the end of the chain) if the cursor is currently
+   * pointing at the end of a buffer.
    */
   size_t length() const {
     return crtBuf_->length() - offset_;
@@ -300,15 +300,26 @@ class CursorBase {
   /**
    * Return the available data in the current buffer.
    * If you want to gather more data from the chain into a contiguous region
-   * (for hopefully zero-copy access), use gather() before peek().
+   * (for hopefully zero-copy access), use gather() before peekBytes().
    */
-  std::pair<const uint8_t*, size_t> peek() {
+  ByteRange peekBytes() {
     // Ensure that we're pointing to valid data
     size_t available = length();
     while (UNLIKELY(available == 0 && tryAdvanceBuffer())) {
       available = length();
     }
-    return std::make_pair(data(), available);
+    return ByteRange{data(), available};
+  }
+
+  /**
+   * Alternate version of peekBytes() that returns a std::pair
+   * instead of a ByteRange.  (This method pre-dates ByteRange.)
+   *
+   * This function will eventually be deprecated.
+   */
+  std::pair<const uint8_t*, size_t> peek() {
+    auto bytes = peekBytes();
+    return std::make_pair(bytes.data(), bytes.size());
   }
 
   void clone(std::unique_ptr<folly::IOBuf>& buf, size_t len) {
@@ -583,9 +594,9 @@ class Writable {
   size_t pushAtMost(Cursor cursor, size_t len) {
     size_t written = 0;
     for(;;) {
-      auto currentBuffer = cursor.peek();
-      const uint8_t* crtData = currentBuffer.first;
-      size_t available = currentBuffer.second;
+      auto currentBuffer = cursor.peekBytes();
+      const uint8_t* crtData = currentBuffer.data();
+      size_t available = currentBuffer.size();
       if (available == 0) {
         // end of buffer chain
         return written;

diff --git a/folly/io/IOBuf.cpp b/folly/io/IOBuf.cpp
index 603e75f745cb1d1dbf806a90f34d1caa47849a7d..763522b74abc1b2134c9d20966a522f69f0641af 100644 (file)
--- a/folly/io/IOBuf.cpp
+++ b/folly/io/IOBuf.cpp
@@ -962,12 +962,12 @@ size_t IOBufHash::operator()(const IOBuf& buf) const {
   hasher.Init(0, 0);
   io::Cursor cursor(&buf);
   for (;;) {
-    auto p = cursor.peek();
-    if (p.second == 0) {
+    auto b = cursor.peekBytes();
+    if (b.empty()) {
       break;
     }
-    hasher.Update(p.first, p.second);
-    cursor.skip(p.second);
+    hasher.Update(b.data(), b.size());
+    cursor.skip(b.size());
   }
   uint64_t h1;
   uint64_t h2;
@@ -979,16 +979,16 @@ bool IOBufEqual::operator()(const IOBuf& a, const IOBuf& b) const {
   io::Cursor ca(&a);
   io::Cursor cb(&b);
   for (;;) {
-    auto pa = ca.peek();
-    auto pb = cb.peek();
-    if (pa.second == 0 && pb.second == 0) {
+    auto ba = ca.peekBytes();
+    auto bb = cb.peekBytes();
+    if (ba.empty() && bb.empty()) {
       return true;
-    } else if (pa.second == 0 || pb.second == 0) {
+    } else if (ba.empty() || bb.empty()) {
       return false;
     }
-    size_t n = std::min(pa.second, pb.second);
+    size_t n = std::min(ba.size(), bb.size());
     DCHECK_GT(n, 0);
-    if (memcmp(pa.first, pb.first, n)) {
+    if (memcmp(ba.data(), bb.data(), n)) {
       return false;
     }
     ca.skip(n);

diff --git a/folly/io/test/IOBufCursorBenchmark.cpp b/folly/io/test/IOBufCursorBenchmark.cpp
index 233f800b2921bb0a5c4e5f1d60edbc091a87f9c8..559f7a3eb972b71fad2a3442f07140fc9df1e465 100644 (file)
--- a/folly/io/test/IOBufCursorBenchmark.cpp
+++ b/folly/io/test/IOBufCursorBenchmark.cpp
@@ -70,7 +70,7 @@ BENCHMARK(skipBenchmark, iters) {
   while (iters--) {
     Cursor c(iobuf_read_benchmark.get());
     for (int i = 0; i < benchmark_size; i++) {
-      c.peek();
+      c.peekBytes();
       c.skip(1);
     }
   }

diff --git a/folly/io/test/IOBufCursorTest.cpp b/folly/io/test/IOBufCursorTest.cpp
index ab8d1f83ed18224b20d8d7080d0e4b98bcf7e8c7..958f6d4589fea1cddf8ce1fa87dcb5537aed91f7 100644 (file)
--- a/folly/io/test/IOBufCursorTest.cpp
+++ b/folly/io/test/IOBufCursorTest.cpp
@@ -182,10 +182,10 @@ void append(Appender& appender, StringPiece data) {
 std::string toString(const IOBuf& buf) {
   std::string str;
   Cursor cursor(&buf);
-  std::pair<const uint8_t*, size_t> p;
-  while ((p = cursor.peek()).second) {
-    str.append(reinterpret_cast<const char*>(p.first), p.second);
-    cursor.skip(p.second);
+  ByteRange b;
+  while (!(b = cursor.peekBytes()).empty()) {
+    str.append(reinterpret_cast<const char*>(b.data()), b.size());
+    cursor.skip(b.size());
   }
   return str;
 }
@@ -218,18 +218,15 @@ TEST(IOBuf, PullAndPeek) {
 
   {
     RWPrivateCursor cursor(iobuf1.get());
-    auto p = cursor.peek();
-    EXPECT_EQ("he", std::string(reinterpret_cast<const char*>(p.first),
-                                p.second));
-    cursor.skip(p.second);
-    p = cursor.peek();
-    EXPECT_EQ("llo ", std::string(reinterpret_cast<const char*>(p.first),
-                                  p.second));
-    cursor.skip(p.second);
-    p = cursor.peek();
-    EXPECT_EQ("world", std::string(reinterpret_cast<const char*>(p.first),
-                                   p.second));
-    cursor.skip(p.second);
+    auto b = cursor.peekBytes();
+    EXPECT_EQ("he", StringPiece(b));
+    cursor.skip(b.size());
+    b = cursor.peekBytes();
+    EXPECT_EQ("llo ", StringPiece(b));
+    cursor.skip(b.size());
+    b = cursor.peekBytes();
+    EXPECT_EQ("world", StringPiece(b));
+    cursor.skip(b.size());
     EXPECT_EQ(3, iobuf1->countChainElements());
     EXPECT_EQ(11, iobuf1->computeChainDataLength());
   }
@@ -237,9 +234,8 @@ TEST(IOBuf, PullAndPeek) {
   {
     RWPrivateCursor cursor(iobuf1.get());
     cursor.gather(11);
-    auto p = cursor.peek();
-    EXPECT_EQ("hello world", std::string(reinterpret_cast<const
-                                         char*>(p.first), p.second));
+    auto b = cursor.peekBytes();
+    EXPECT_EQ("hello world", StringPiece(b));
     EXPECT_EQ(1, iobuf1->countChainElements());
     EXPECT_EQ(11, iobuf1->computeChainDataLength());
   }
@@ -377,7 +373,7 @@ TEST(IOBuf, cloneAndInsert) {
     EXPECT_EQ(7, iobuf1->countChainElements());
     EXPECT_EQ(14, iobuf1->computeChainDataLength());
     // Check that nextBuf got set correctly to the buffer with 1 byte left
-    EXPECT_EQ(1, cursor.peek().second);
+    EXPECT_EQ(1, cursor.peekBytes().size());
     cursor.read<uint8_t>();
   }