commit e3dab2d906ee45954e58826b11cce9219c005cc5
author Yi Jin <jinyithu@google.com> Thu Mar 22 16:56:39 2018 -0700
committer Yi Jin <jinyithu@google.com> Wed Mar 28 12:44:36 2018 -0700
tree 5498ede2ec95787f958469b6ed1bc8b809caa047
parent ab34199e9c1da603788893537107552ef6245712

Use unique_fd with the clear ownership.

FdBuffer won't take ownership of the `main` fd.
It only enforces transfer ownership in readProcessedDataInStream.

Bug: 74021345
Test: atest incidentd_test
Change-Id: I6182730241c81c34b3be865b827a2d3e8c10c21c

cmds/incidentd/src/Section.cpp

diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 3b57d34..66e747c 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -264,8 +264,9 @@
     }
 
     // parent process
-    status_t readStatus = buffer.readProcessedDataInStream(
-            &fd, &p2cPipe.writeFd(), &c2pPipe.readFd(), this->timeoutMs, mIsSysfs);
+    status_t readStatus = buffer.readProcessedDataInStream(fd.get(), std::move(p2cPipe.writeFd()),
+                                                           std::move(c2pPipe.readFd()),
+                                                           this->timeoutMs, mIsSysfs);
 
     if (readStatus != NO_ERROR || buffer.timedOut()) {
         ALOGW("FileSection '%s' failed to read data from incident helper: %s, timedout: %s",
@@ -354,9 +355,9 @@
     VLOG("GZipSection '%s' editPos=%zd, dataBeginAt=%zd", this->name.string(), editPos,
          dataBeginAt);
 
-    status_t readStatus =
-            buffer.readProcessedDataInStream(&fd, &p2cPipe.writeFd(), &c2pPipe.readFd(),
-                                             this->timeoutMs, isSysfs(mFilenames[index]));
+    status_t readStatus = buffer.readProcessedDataInStream(
+            fd.get(), std::move(p2cPipe.writeFd()), std::move(c2pPipe.readFd()), this->timeoutMs,
+            isSysfs(mFilenames[index]));
 
     if (readStatus != NO_ERROR || buffer.timedOut()) {
         ALOGW("GZipSection '%s' failed to read data from gzip: %s, timedout: %s",
@@ -466,7 +467,7 @@
     pthread_attr_destroy(&attr);
 
     // Loop reading until either the timeout or the worker side is done (i.e. eof).
-    err = buffer.read(&data->pipe.readFd(), this->timeoutMs);
+    err = buffer.read(data->pipe.readFd().get(), this->timeoutMs);
     if (err != NO_ERROR) {
         // TODO: Log this error into the incident report.
         ALOGW("WorkerThreadSection '%s' reader failed with error '%s'", this->name.string(),
@@ -573,7 +574,7 @@
     }
 
     cmdPipe.writeFd().reset();
-    status_t readStatus = buffer.read(&ihPipe.readFd(), this->timeoutMs);
+    status_t readStatus = buffer.read(ihPipe.readFd().get(), this->timeoutMs);
     if (readStatus != NO_ERROR || buffer.timedOut()) {
         ALOGW("CommandSection '%s' failed to read data from incident helper: %s, timedout: %s",
               this->name.string(), strerror(-readStatus), buffer.timedOut() ? "true" : "false");
@@ -892,7 +893,7 @@
         // Parent process.
         // Read from the pipe concurrently to avoid blocking the child.
         FdBuffer buffer;
-        err = buffer.readFully(&dumpPipe.readFd());
+        err = buffer.readFully(dumpPipe.readFd().get());
         if (err != NO_ERROR) {
             ALOGW("TombstoneSection '%s' failed to read stack dump: %d", this->name.string(), err);
             dumpPipe.readFd().reset();