You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2013/01/31 06:06:10 UTC

svn commit: r1440848 - in /incubator/mesos/trunk: src/tests/protobuf_io_tests.cpp third_party/libprocess/include/stout/protobuf.hpp

Author: benh
Date: Thu Jan 31 05:06:10 2013
New Revision: 1440848

URL: http://svn.apache.org/viewvc?rev=1440848&view=rev
Log:
Refactored protobuf::read to return the protobuf (instead of bool).

From: Ben Mahler <be...@gmail.com>
Review: https://reviews.apache.org/r/8696

Modified:
    incubator/mesos/trunk/src/tests/protobuf_io_tests.cpp
    incubator/mesos/trunk/third_party/libprocess/include/stout/protobuf.hpp

Modified: incubator/mesos/trunk/src/tests/protobuf_io_tests.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/tests/protobuf_io_tests.cpp?rev=1440848&r1=1440847&r2=1440848&view=diff
==============================================================================
--- incubator/mesos/trunk/src/tests/protobuf_io_tests.cpp (original)
+++ incubator/mesos/trunk/src/tests/protobuf_io_tests.cpp Thu Jan 31 05:06:10 2013
@@ -58,17 +58,15 @@ TEST(ProtobufIOTest, Basic)
     ASSERT_SOME(result);
   }
 
-  Result<bool> read = Result<bool>::none();
+  Result<FrameworkID> read = Result<FrameworkID>::none();
   size_t reads = 0;
   while (true) {
-    FrameworkID frameworkId;
-    read = protobuf::read(fdr, &frameworkId);
+    read = protobuf::read<FrameworkID>(fdr);
     if (!read.isSome()) {
       break;
     }
 
-    EXPECT_TRUE(read.get());
-    EXPECT_EQ(frameworkId.value(), stringify(reads++));
+    EXPECT_EQ(read.get().value(), stringify(reads++));
   }
 
   // Ensure we've hit the end of the file without reading a partial protobuf.

Modified: incubator/mesos/trunk/third_party/libprocess/include/stout/protobuf.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/third_party/libprocess/include/stout/protobuf.hpp?rev=1440848&r1=1440847&r2=1440848&view=diff
==============================================================================
--- incubator/mesos/trunk/third_party/libprocess/include/stout/protobuf.hpp (original)
+++ incubator/mesos/trunk/third_party/libprocess/include/stout/protobuf.hpp Thu Jan 31 05:06:10 2013
@@ -80,18 +80,15 @@ inline Try<Nothing> write(
 }
 
 
-// Read the next protobuf from the file by first reading the "size"
+// Read the next protobuf of type T from the file by first reading the "size"
 // followed by the contents (as written by 'write' above).
-inline Result<bool> read(int fd, google::protobuf::Message* message)
+template <typename T>
+inline Result<T> read(int fd)
 {
-  CHECK_NOTNULL(message);
-
-  message->Clear();
-
   // Save the offset so we can re-adjust if something goes wrong.
   off_t offset = lseek(fd, 0, SEEK_CUR);
   if (offset == -1) {
-    return Result<bool>::error(
+    return Result<T>::error(
         "Failed to lseek to SEEK_CUR: " + std::string(strerror(errno)));
   }
 
@@ -99,10 +96,9 @@ inline Result<bool> read(int fd, google:
   Result<std::string> result = os::read(fd, sizeof(size));
 
   if (result.isNone()) {
-    return Result<bool>::none(); // No more protobufs to read.
+    return Result<T>::none(); // No more protobufs to read.
   } else if (result.isError()) {
-    return Result<bool>::error(
-        "Failed to read protobuf size: " + result.error());
+    return Result<T>::error("Failed to read protobuf size: " + result.error());
   }
 
   // Parse the size from the bytes.
@@ -116,39 +112,39 @@ inline Result<bool> read(int fd, google:
   if (result.isNone()) {
     // Hit EOF unexpectedly. Restore the offset to before the size read.
     lseek(fd, offset, SEEK_SET);
-    return Result<bool>::error(
+    return Result<T>::error(
         "Failed to read protobuf of size " + stringify(size) + " bytes: "
         "hit EOF unexpectedly, possible corruption");
   } else if (result.isError()) {
     // Restore the offset to before the size read.
     lseek(fd, offset, SEEK_SET);
-    return Result<bool>::error("Failed to read protobuf: " + result.error());
+    return Result<T>::error("Failed to read protobuf: " + result.error());
   }
 
   // Parse the protobuf from the string.
+  T message;
   google::protobuf::io::ArrayInputStream stream(
       result.get().data(), result.get().size());
-  bool parsed = message->ParseFromZeroCopyStream(&stream);
+  bool parsed = message.ParseFromZeroCopyStream(&stream);
 
   if (!parsed) {
     // Restore the offset to before the size read.
     lseek(fd, offset, SEEK_SET);
     // NOTE: It appears that ParseFromZeroCopyStream will keep
     // errno intact on failure, but this may change.
-    return Result<bool>::error(
+    return Result<T>::error(
         "Failed to ParseFromZeroCopyStream, possible strerror: " +
         std::string(strerror(errno)));
   }
 
-  return true;
+  return message;
 }
 
 
 // A wrapper function that wraps the above read() with
 // open and closing the file.
-inline Result<bool> read(
-    const std::string& path,
-    google::protobuf::Message* message)
+template <typename T>
+inline Result<T> read(const std::string& path)
 {
   Try<int> fd = os::open(
       path, O_RDONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IRWXO);
@@ -157,7 +153,7 @@ inline Result<bool> read(
     return Result<bool>::error("Failed to open file " + path);
   }
 
-  Result<bool> result = read(fd.get(), message);
+  Result<T> result = read<T>(fd.get());
 
   // NOTE: We ignore the return value of close(). This is because users calling
   // this function are interested in the return value of read(). Also an