Perfprofd: Use a stream to serialize

Move from a memory buffer to streaming to file.

Bug: 73175642
Test: mmma system/extras/perfprofd
Test: perfprofd_test
Change-Id: Ibb37b8942e7dbf3c5ff9bf69c00473c57f7d2a7c

diff --git a/perfprofd/binder_interface/perfprofd_binder.cc b/perfprofd/binder_interface/perfprofd_binder.cc
index 64d216d..d04e185 100644 (file)
--- a/perfprofd/binder_interface/perfprofd_binder.cc
+++ b/perfprofd/binder_interface/perfprofd_binder.cc
@@ -34,7 +34,6 @@
 #include <android-base/file.h>
 #include <android-base/logging.h>
 #include <android-base/stringprintf.h>
-#include <android-base/test_utils.h>
 #include <android-base/unique_fd.h>
 #include <android/os/DropBoxManager.h>
 #include <binder/BinderService.h>
@@ -154,32 +153,22 @@ class PerfProfdNativeService : public BinderService<PerfProfdNativeService>,
 
 static Status WriteDropboxFile(android::perfprofd::PerfprofdRecord* encodedProfile,
                                Config* config) {
-  TemporaryFile tmp_file(config->destination_directory);
-  // Remove the temp file from the file system for security reasons.
-  tmp_file.DoNotRemove();
-  if (unlink(tmp_file.path) != 0) {
-    PLOG(WARNING) << "Could not unlink binder temp file";
-  }
-
+  android::base::unique_fd tmp_fd;
   {
-    // protobuf-lite doesn't have file streams.
-    struct FileCopyingOutputStream : public google::protobuf::io::CopyingOutputStream {
-      int fd;
-      explicit FileCopyingOutputStream(int fd_in) : fd(fd_in) {
-      }
-      bool Write(const void * buffer, int size) override {
-        return android::base::WriteFully(fd, buffer, size);
-      }
-    };
-    FileCopyingOutputStream fcos(tmp_file.fd);
-    google::protobuf::io::CopyingOutputStreamAdaptor cosa(&fcos);
-    bool serialized = encodedProfile->SerializeToZeroCopyStream(&cosa);
-    if (!serialized) {
-      return Status::fromExceptionCode(1, "Failed to serialize proto");
+    char path[PATH_MAX];
+    snprintf(path,
+             sizeof(path),
+             "%s%cdropboxtmp-XXXXXX",
+             config->destination_directory.c_str(),
+             OS_PATH_SEPARATOR);
+    tmp_fd.reset(mkstemp(path));
+    if (tmp_fd.get() == -1) {
+      PLOG(ERROR) << "Could not create temp file " << path;
+      return Status::fromExceptionCode(1, "Could not create temp file");
+    }
+    if (unlink(path) != 0) {
+      PLOG(WARNING) << "Could not unlink binder temp file";
     }
-    cosa.Flush();
-
-    // TODO: Is an fsync necessary here?
   }
 
   // Dropbox takes ownership of the fd, and if it is not readonly,
@@ -187,7 +176,7 @@ static Status WriteDropboxFile(android::perfprofd::PerfprofdRecord* encodedProfi
   android::base::unique_fd read_only;
   {
     char fdpath[64];
-    snprintf(fdpath, arraysize(fdpath), "/proc/self/fd/%d", tmp_file.fd);
+    snprintf(fdpath, arraysize(fdpath), "/proc/self/fd/%d", tmp_fd.get());
     read_only.reset(open(fdpath, O_RDONLY | O_CLOEXEC));
     if (read_only.get() < 0) {
       PLOG(ERROR) << "Could not create read-only fd";
@@ -195,6 +184,10 @@ static Status WriteDropboxFile(android::perfprofd::PerfprofdRecord* encodedProfi
     }
   }
 
+  if (!SerializeProtobuf(encodedProfile, std::move(tmp_fd))) {
+    return Status::fromExceptionCode(1, "Could not serialize to temp file");
+  }
+
   using DropBoxManager = android::os::DropBoxManager;
   sp<DropBoxManager> dropbox(new DropBoxManager());
   return dropbox->addFile(String16("perfprofd"), read_only.release(), 0);

diff --git a/perfprofd/perfprofd_io.cc b/perfprofd/perfprofd_io.cc
index 202ee2e..0b0bd6d 100644 (file)
--- a/perfprofd/perfprofd_io.cc
+++ b/perfprofd/perfprofd_io.cc
@@ -20,42 +20,59 @@
 #include <fcntl.h>
 #include <unistd.h>
 
-#include <memory>
-
 #include <android-base/file.h>
 #include <android-base/logging.h>
-#include <android-base/unique_fd.h>
+#include <google/protobuf/io/zero_copy_stream_impl_lite.h>
 
 #include "perfprofd_record.pb.h"
 
 namespace android {
 namespace perfprofd {
 
-bool SerializeProtobuf(PerfprofdRecord* encodedProfile,
-                       const char* encoded_file_path) {
-  //
-  // Serialize protobuf to array
-  //
-  size_t size = encodedProfile->ByteSize();
-  std::unique_ptr<uint8_t[]> data(new uint8_t[size]);
-  encodedProfile->SerializeWithCachedSizesToArray(data.get());
-
-  //
-  // Open file and write encoded data to it
-  //
+using android::base::unique_fd;
+using android::base::WriteFully;
+
+namespace {
+
+// Protobuf's file implementation is not available in protobuf-lite. :-(
+class FileCopyingOutputStream : public ::google::protobuf::io::CopyingOutputStream {
+ public:
+  explicit FileCopyingOutputStream(android::base::unique_fd&& fd_in) : fd_(std::move(fd_in)) {
+  };
+  bool Write(const void * buffer, int size) override {
+    return WriteFully(fd_.get(), buffer, size);
+  }
+
+ private:
+  android::base::unique_fd fd_;
+};
+
+}  // namespace
+
+bool SerializeProtobuf(android::perfprofd::PerfprofdRecord* encodedProfile,
+                       android::base::unique_fd&& fd) {
+  FileCopyingOutputStream fcos(std::move(fd));
+  google::protobuf::io::CopyingOutputStreamAdaptor cosa(&fcos);
+
+  bool serialized = encodedProfile->SerializeToZeroCopyStream(&cosa);
+  if (!serialized) {
+    LOG(WARNING) << "SerializeToZeroCopyStream failed";
+    return false;
+  }
+
+  cosa.Flush();
+  return true;
+}
+
+bool SerializeProtobuf(PerfprofdRecord* encodedProfile, const char* encoded_file_path) {
   unlink(encoded_file_path);  // Attempt to unlink for a clean slate.
   constexpr int kFlags = O_CREAT | O_WRONLY | O_TRUNC | O_NOFOLLOW | O_CLOEXEC;
-  android::base::unique_fd fd(open(encoded_file_path, kFlags, 0664));
+  unique_fd fd(open(encoded_file_path, kFlags, 0664));
   if (fd.get() == -1) {
     PLOG(WARNING) << "Could not open " << encoded_file_path << " for serialization";
     return false;
   }
-  if (!android::base::WriteFully(fd.get(), data.get(), size)) {
-    PLOG(WARNING) << "Could not write to " << encoded_file_path;
-    return false;
-  }
-
-  return true;
+  return SerializeProtobuf(encodedProfile, std::move(fd));
 }
 
 }  // namespace perfprofd

diff --git a/perfprofd/perfprofd_io.h b/perfprofd/perfprofd_io.h
index 7387956..a102cd8 100644 (file)
--- a/perfprofd/perfprofd_io.h
+++ b/perfprofd/perfprofd_io.h
@@ -18,6 +18,8 @@
 #ifndef SYSTEM_EXTRAS_PERFPROFD_PERFPROFD_IO_H_
 #define SYSTEM_EXTRAS_PERFPROFD_PERFPROFD_IO_H_
 
+#include <android-base/unique_fd.h>
+
 #include "perfprofd_record-fwd.h"
 
 namespace android {
@@ -25,6 +27,8 @@ namespace perfprofd {
 
 bool SerializeProtobuf(android::perfprofd::PerfprofdRecord* encodedProfile,
                        const char* encoded_file_path);
+bool SerializeProtobuf(android::perfprofd::PerfprofdRecord* encodedProfile,
+                       android::base::unique_fd&& fd);
 
 }  // namespace perfprofd
 }  // namespace android