Wrap fd with unique_fd so it won't leak.
Bug: 74021345
Test: manual and atest incidentd_test
Change-Id: Ib1000bfe6917c3d5cae7b9edce5b67d50897e10d
diff --git a/cmds/incidentd/src/FdBuffer.cpp b/cmds/incidentd/src/FdBuffer.cpp
index 3570144..2b85ec0 100644
--- a/cmds/incidentd/src/FdBuffer.cpp
+++ b/cmds/incidentd/src/FdBuffer.cpp
@@ -34,11 +34,11 @@
FdBuffer::~FdBuffer() {}
-status_t FdBuffer::read(int fd, int64_t timeout) {
- struct pollfd pfds = {.fd = fd, .events = POLLIN};
+status_t FdBuffer::read(unique_fd* fd, int64_t timeout) {
+ struct pollfd pfds = {.fd = fd->get(), .events = POLLIN};
mStartTime = uptimeMillis();
- fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK);
+ fcntl(fd->get(), F_SETFL, fcntl(fd->get(), F_GETFL, 0) | O_NONBLOCK);
while (true) {
if (mBuffer.size() >= MAX_BUFFER_COUNT * BUFFER_SIZE) {
@@ -67,16 +67,16 @@
VLOG("return event has error %s", strerror(errno));
return errno != 0 ? -errno : UNKNOWN_ERROR;
} else {
- ssize_t amt = ::read(fd, mBuffer.writeBuffer(), mBuffer.currentToWrite());
+ ssize_t amt = ::read(fd->get(), mBuffer.writeBuffer(), mBuffer.currentToWrite());
if (amt < 0) {
if (errno == EAGAIN || errno == EWOULDBLOCK) {
continue;
} else {
- VLOG("Fail to read %d: %s", fd, strerror(errno));
+ VLOG("Fail to read %d: %s", fd->get(), strerror(errno));
return -errno;
}
} else if (amt == 0) {
- VLOG("Reached EOF of fd=%d", fd);
+ VLOG("Reached EOF of fd=%d", fd->get());
break;
}
mBuffer.wp()->move(amt);
@@ -87,7 +87,7 @@
return NO_ERROR;
}
-status_t FdBuffer::readFully(int fd) {
+status_t FdBuffer::readFully(unique_fd* fd) {
mStartTime = uptimeMillis();
while (true) {
@@ -99,10 +99,10 @@
}
if (mBuffer.writeBuffer() == NULL) return NO_MEMORY;
- ssize_t amt =
- TEMP_FAILURE_RETRY(::read(fd, mBuffer.writeBuffer(), mBuffer.currentToWrite()));
+ ssize_t amt = TEMP_FAILURE_RETRY(
+ ::read(fd->get(), mBuffer.writeBuffer(), mBuffer.currentToWrite()));
if (amt < 0) {
- VLOG("Fail to read %d: %s", fd, strerror(errno));
+ VLOG("Fail to read %d: %s", fd->get(), strerror(errno));
return -errno;
} else if (amt == 0) {
VLOG("Done reading %zu bytes", mBuffer.size());
@@ -116,20 +116,20 @@
return NO_ERROR;
}
-status_t FdBuffer::readProcessedDataInStream(int fd, int toFd, int fromFd, int64_t timeoutMs,
- const bool isSysfs) {
+status_t FdBuffer::readProcessedDataInStream(unique_fd* fd, unique_fd* toFd, unique_fd* fromFd,
+ int64_t timeoutMs, const bool isSysfs) {
struct pollfd pfds[] = {
- {.fd = fd, .events = POLLIN},
- {.fd = toFd, .events = POLLOUT},
- {.fd = fromFd, .events = POLLIN},
+ {.fd = fd->get(), .events = POLLIN},
+ {.fd = toFd->get(), .events = POLLOUT},
+ {.fd = fromFd->get(), .events = POLLIN},
};
mStartTime = uptimeMillis();
// mark all fds non blocking
- fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK);
- fcntl(toFd, F_SETFL, fcntl(toFd, F_GETFL, 0) | O_NONBLOCK);
- fcntl(fromFd, F_SETFL, fcntl(fromFd, F_GETFL, 0) | O_NONBLOCK);
+ fcntl(fd->get(), F_SETFL, fcntl(fd->get(), F_GETFL, 0) | O_NONBLOCK);
+ fcntl(toFd->get(), F_SETFL, fcntl(toFd->get(), F_GETFL, 0) | O_NONBLOCK);
+ fcntl(fromFd->get(), F_SETFL, fcntl(fromFd->get(), F_GETFL, 0) | O_NONBLOCK);
// A circular buffer holds data read from fd and writes to parsing process
uint8_t cirBuf[BUFFER_SIZE];
@@ -166,10 +166,10 @@
for (int i = 0; i < 3; ++i) {
if ((pfds[i].revents & POLLERR) != 0) {
if (i == 0 && isSysfs) {
- VLOG("fd %d is sysfs, ignore its POLLERR return value", fd);
+ VLOG("fd %d is sysfs, ignore its POLLERR return value", fd->get());
continue;
}
- VLOG("fd[%d]=%d returns error events: %s", i, fd, strerror(errno));
+ VLOG("fd[%d]=%d returns error events: %s", i, fd->get(), strerror(errno));
return errno != 0 ? -errno : UNKNOWN_ERROR;
}
}
@@ -178,17 +178,17 @@
if (cirSize != BUFFER_SIZE && pfds[0].fd != -1) {
ssize_t amt;
if (rpos >= wpos) {
- amt = ::read(fd, cirBuf + rpos, BUFFER_SIZE - rpos);
+ amt = ::read(fd->get(), cirBuf + rpos, BUFFER_SIZE - rpos);
} else {
- amt = ::read(fd, cirBuf + rpos, wpos - rpos);
+ amt = ::read(fd->get(), cirBuf + rpos, wpos - rpos);
}
if (amt < 0) {
if (!(errno == EAGAIN || errno == EWOULDBLOCK)) {
- VLOG("Fail to read fd %d: %s", fd, strerror(errno));
+ VLOG("Fail to read fd %d: %s", fd->get(), strerror(errno));
return -errno;
} // otherwise just continue
} else if (amt == 0) {
- VLOG("Reached EOF of input file %d", fd);
+ VLOG("Reached EOF of input file %d", fd->get());
pfds[0].fd = -1; // reach EOF so don't have to poll pfds[0].
} else {
rpos += amt;
@@ -200,13 +200,13 @@
if (cirSize > 0 && pfds[1].fd != -1) {
ssize_t amt;
if (rpos > wpos) {
- amt = ::write(toFd, cirBuf + wpos, rpos - wpos);
+ amt = ::write(toFd->get(), cirBuf + wpos, rpos - wpos);
} else {
- amt = ::write(toFd, cirBuf + wpos, BUFFER_SIZE - wpos);
+ amt = ::write(toFd->get(), cirBuf + wpos, BUFFER_SIZE - wpos);
}
if (amt < 0) {
if (!(errno == EAGAIN || errno == EWOULDBLOCK)) {
- VLOG("Fail to write toFd %d: %s", toFd, strerror(errno));
+ VLOG("Fail to write toFd %d: %s", toFd->get(), strerror(errno));
return -errno;
} // otherwise just continue
} else {
@@ -217,8 +217,8 @@
// if buffer is empty and fd is closed, close write fd.
if (cirSize == 0 && pfds[0].fd == -1 && pfds[1].fd != -1) {
- VLOG("Close write pipe %d", toFd);
- ::close(pfds[1].fd);
+ VLOG("Close write pipe %d", toFd->get());
+ toFd->reset();
pfds[1].fd = -1;
}
@@ -231,14 +231,14 @@
}
// read from parsing process
- ssize_t amt = ::read(fromFd, mBuffer.writeBuffer(), mBuffer.currentToWrite());
+ ssize_t amt = ::read(fromFd->get(), mBuffer.writeBuffer(), mBuffer.currentToWrite());
if (amt < 0) {
if (!(errno == EAGAIN || errno == EWOULDBLOCK)) {
- VLOG("Fail to read fromFd %d: %s", fromFd, strerror(errno));
+ VLOG("Fail to read fromFd %d: %s", fromFd->get(), strerror(errno));
return -errno;
} // otherwise just continue
} else if (amt == 0) {
- VLOG("Reached EOF of fromFd %d", fromFd);
+ VLOG("Reached EOF of fromFd %d", fromFd->get());
break;
} else {
mBuffer.wp()->move(amt);
diff --git a/cmds/incidentd/src/FdBuffer.h b/cmds/incidentd/src/FdBuffer.h
index 34ebcf5..db3a74b 100644
--- a/cmds/incidentd/src/FdBuffer.h
+++ b/cmds/incidentd/src/FdBuffer.h
@@ -18,10 +18,12 @@
#ifndef FD_BUFFER_H
#define FD_BUFFER_H
+#include <android-base/unique_fd.h>
#include <android/util/EncodedBuffer.h>
#include <utils/Errors.h>
using namespace android;
+using namespace android::base;
using namespace android::util;
using namespace std;
@@ -38,13 +40,13 @@
* Returns NO_ERROR if there were no errors or if we timed out.
* Will mark the file O_NONBLOCK.
*/
- status_t read(int fd, int64_t timeoutMs);
+ status_t read(unique_fd* fd, int64_t timeoutMs);
/**
* Read the data until we hit eof.
* Returns NO_ERROR if there were no errors.
*/
- status_t readFully(int fd);
+ status_t readFully(unique_fd* fd);
/**
* Read processed results by streaming data to a parsing process, e.g. incident helper.
@@ -56,8 +58,8 @@
*
* Poll will return POLLERR if fd is from sysfs, handle this edge case.
*/
- status_t readProcessedDataInStream(int fd, int toFd, int fromFd, int64_t timeoutMs,
- const bool isSysfs = false);
+ status_t readProcessedDataInStream(unique_fd* fd, unique_fd* toFd, unique_fd* fromFd,
+ int64_t timeoutMs, const bool isSysfs = false);
/**
* Whether we timed out.
diff --git a/cmds/incidentd/src/IncidentService.cpp b/cmds/incidentd/src/IncidentService.cpp
index d02b4dd..aeccefd 100644
--- a/cmds/incidentd/src/IncidentService.cpp
+++ b/cmds/incidentd/src/IncidentService.cpp
@@ -352,7 +352,8 @@
printPrivacy(p, out, String8(""));
} else if (opt == "parse") {
FdBuffer buf;
- status_t error = buf.read(fileno(in), 60000);
+ unique_fd infd(fileno(in));
+ status_t error = buf.read(&infd, 60000);
if (error != NO_ERROR) {
fprintf(err, "Error reading from stdin\n");
return error;
diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 5cde5a9..ab4e764 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -277,8 +277,8 @@
status_t FileSection::Execute(ReportRequestSet* requests) const {
// read from mFilename first, make sure the file is available
// add O_CLOEXEC to make sure it is closed when exec incident helper
- int fd = open(mFilename, O_RDONLY | O_CLOEXEC);
- if (fd == -1) {
+ unique_fd fd(open(mFilename, O_RDONLY | O_CLOEXEC));
+ if (fd.get() == -1) {
ALOGW("FileSection '%s' failed to open file", this->name.string());
return -errno;
}
@@ -299,9 +299,8 @@
}
// parent process
- status_t readStatus = buffer.readProcessedDataInStream(fd, p2cPipe.writeFd(), c2pPipe.readFd(),
- this->timeoutMs, mIsSysfs);
- close(fd); // close the fd anyway.
+ status_t readStatus = buffer.readProcessedDataInStream(
+ &fd, &p2cPipe.writeFd(), &c2pPipe.readFd(), this->timeoutMs, mIsSysfs);
if (readStatus != NO_ERROR || buffer.timedOut()) {
ALOGW("FileSection '%s' failed to read data from incident helper: %s, timedout: %s",
@@ -342,17 +341,17 @@
status_t GZipSection::Execute(ReportRequestSet* requests) const {
// Reads the files in order, use the first available one.
int index = 0;
- int fd = -1;
+ unique_fd fd;
while (mFilenames[index] != NULL) {
- fd = open(mFilenames[index], O_RDONLY | O_CLOEXEC);
- if (fd != -1) {
+ fd.reset(open(mFilenames[index], O_RDONLY | O_CLOEXEC));
+ if (fd.get() != -1) {
break;
}
ALOGW("GZipSection failed to open file %s", mFilenames[index]);
index++; // look at the next file.
}
- VLOG("GZipSection is using file %s, fd=%d", mFilenames[index], fd);
- if (fd == -1) return -1;
+ VLOG("GZipSection is using file %s, fd=%d", mFilenames[index], fd.get());
+ if (fd.get() == -1) return -1;
FdBuffer buffer;
Fpipe p2cPipe;
@@ -388,9 +387,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]));
- close(fd); // close the fd anyway.
+ status_t readStatus =
+ buffer.readProcessedDataInStream(&fd, &p2cPipe.writeFd(), &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",
@@ -424,7 +423,7 @@
// ================================================================================
struct WorkerThreadData : public virtual RefBase {
const WorkerThreadSection* section;
- int fds[2];
+ Fpipe pipe;
// Lock protects these fields
mutex lock;
@@ -433,16 +432,10 @@
WorkerThreadData(const WorkerThreadSection* section);
virtual ~WorkerThreadData();
-
- int readFd() { return fds[0]; }
- int writeFd() { return fds[1]; }
};
WorkerThreadData::WorkerThreadData(const WorkerThreadSection* sec)
- : section(sec), workerDone(false), workerError(NO_ERROR) {
- fds[0] = -1;
- fds[1] = -1;
-}
+ : section(sec), workerDone(false), workerError(NO_ERROR) {}
WorkerThreadData::~WorkerThreadData() {}
@@ -454,7 +447,7 @@
static void* worker_thread_func(void* cookie) {
WorkerThreadData* data = (WorkerThreadData*)cookie;
- status_t err = data->section->BlockingCall(data->writeFd());
+ status_t err = data->section->BlockingCall(data->pipe.writeFd().get());
{
unique_lock<mutex> lock(data->lock);
@@ -462,7 +455,7 @@
data->workerError = err;
}
- close(data->writeFd());
+ data->pipe.writeFd().reset();
data->decStrong(data->section);
// data might be gone now. don't use it after this point in this thread.
return NULL;
@@ -479,8 +472,7 @@
sp<WorkerThreadData> data = new WorkerThreadData(this);
// Create the pipe
- err = pipe(data->fds);
- if (err != 0) {
+ if (!data->pipe.init()) {
return -errno;
}
@@ -507,7 +499,7 @@
pthread_attr_destroy(&attr);
// Loop reading until either the timeout or the worker side is done (i.e. eof).
- err = buffer.read(data->readFd(), this->timeoutMs);
+ err = buffer.read(&data->pipe.readFd(), 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(),
@@ -516,7 +508,7 @@
// Done with the read fd. The worker thread closes the write one so
// we never race and get here first.
- close(data->readFd());
+ data->pipe.readFd().reset();
// If the worker side is finished, then return its error (which may overwrite
// our possible error -- but it's more interesting anyway). If not, then we timed out.
@@ -602,7 +594,8 @@
// child process to execute the command as root
if (cmdPid == 0) {
// replace command's stdout with ihPipe's write Fd
- if (dup2(cmdPipe.writeFd(), STDOUT_FILENO) != 1 || !ihPipe.close() || !cmdPipe.close()) {
+ if (dup2(cmdPipe.writeFd().get(), STDOUT_FILENO) != 1 || !ihPipe.close() ||
+ !cmdPipe.close()) {
ALOGW("CommandSection '%s' failed to set up stdout: %s", this->name.string(),
strerror(errno));
_exit(EXIT_FAILURE);
@@ -619,8 +612,8 @@
return -errno;
}
- close(cmdPipe.writeFd());
- status_t readStatus = buffer.read(ihPipe.readFd(), this->timeoutMs);
+ cmdPipe.writeFd().reset();
+ status_t readStatus = buffer.read(&ihPipe.readFd(), 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");
@@ -921,10 +914,10 @@
break;
} else if (child == 0) {
// This is the child process.
- close(dumpPipe.readFd());
+ dumpPipe.readFd().reset();
const int ret = dump_backtrace_to_file_timeout(
pid, is_java_process ? kDebuggerdJavaBacktrace : kDebuggerdNativeBacktrace,
- is_java_process ? 5 : 20, dumpPipe.writeFd());
+ is_java_process ? 5 : 20, dumpPipe.writeFd().get());
if (ret == -1) {
if (errno == 0) {
ALOGW("Dumping failed for pid '%d', likely due to a timeout\n", pid);
@@ -932,25 +925,17 @@
ALOGE("Dumping failed for pid '%d': %s\n", pid, strerror(errno));
}
}
- if (close(dumpPipe.writeFd()) != 0) {
- ALOGW("TombstoneSection '%s' failed to close dump pipe writeFd: %d",
- this->name.string(), errno);
- _exit(EXIT_FAILURE);
- }
-
+ dumpPipe.writeFd().reset();
_exit(EXIT_SUCCESS);
}
- close(dumpPipe.writeFd());
+ dumpPipe.writeFd().reset();
// Parent process.
// Read from the pipe concurrently to avoid blocking the child.
FdBuffer buffer;
- err = buffer.readFully(dumpPipe.readFd());
+ err = buffer.readFully(&dumpPipe.readFd());
if (err != NO_ERROR) {
ALOGW("TombstoneSection '%s' failed to read stack dump: %d", this->name.string(), err);
- if (close(dumpPipe.readFd()) != 0) {
- ALOGW("TombstoneSection '%s' failed to close dump pipe readFd: %s",
- this->name.string(), strerror(errno));
- }
+ dumpPipe.readFd().reset();
break;
}
@@ -967,13 +952,7 @@
proto.write(android::os::BackTraceProto::Stack::DUMP_DURATION_NS,
static_cast<long long>(Nanotime() - start));
proto.end(token);
-
- if (close(dumpPipe.readFd()) != 0) {
- ALOGW("TombstoneSection '%s' failed to close dump pipe readFd: %d", this->name.string(),
- errno);
- err = -errno;
- break;
- }
+ dumpPipe.readFd().reset();
}
proto.flush(pipeWriteFd);
diff --git a/cmds/incidentd/src/incidentd_util.cpp b/cmds/incidentd/src/incidentd_util.cpp
index c869c7a..d799513 100644
--- a/cmds/incidentd/src/incidentd_util.cpp
+++ b/cmds/incidentd/src/incidentd_util.cpp
@@ -53,16 +53,17 @@
bool Fpipe::init() { return Pipe(&mRead, &mWrite); }
-int Fpipe::readFd() const { return mRead.get(); }
+unique_fd& Fpipe::readFd() { return mRead; }
-int Fpipe::writeFd() const { return mWrite.get(); }
+unique_fd& Fpipe::writeFd() { return mWrite; }
pid_t fork_execute_cmd(const char* cmd, char* const argv[], Fpipe* input, Fpipe* output) {
// fork used in multithreaded environment, avoid adding unnecessary code in child process
pid_t pid = fork();
if (pid == 0) {
- if (TEMP_FAILURE_RETRY(dup2(input->readFd(), STDIN_FILENO)) < 0 || !input->close() ||
- TEMP_FAILURE_RETRY(dup2(output->writeFd(), STDOUT_FILENO)) < 0 || !output->close()) {
+ if (TEMP_FAILURE_RETRY(dup2(input->readFd().get(), STDIN_FILENO)) < 0 || !input->close() ||
+ TEMP_FAILURE_RETRY(dup2(output->writeFd().get(), STDOUT_FILENO)) < 0 ||
+ !output->close()) {
ALOGW("Can't setup stdin and stdout for command %s", cmd);
_exit(EXIT_FAILURE);
}
@@ -76,8 +77,8 @@
_exit(EXIT_FAILURE); // always exits with failure if any
}
// close the fds used in child process.
- close(input->readFd());
- close(output->writeFd());
+ input->readFd().reset();
+ output->writeFd().reset();
return pid;
}
diff --git a/cmds/incidentd/src/incidentd_util.h b/cmds/incidentd/src/incidentd_util.h
index 3f7df91..228d776 100644
--- a/cmds/incidentd/src/incidentd_util.h
+++ b/cmds/incidentd/src/incidentd_util.h
@@ -41,8 +41,8 @@
bool init();
bool close();
- int readFd() const;
- int writeFd() const;
+ unique_fd& readFd();
+ unique_fd& writeFd();
private:
unique_fd mRead;
diff --git a/cmds/incidentd/tests/FdBuffer_test.cpp b/cmds/incidentd/tests/FdBuffer_test.cpp
index 0e5eec6..bf77017 100644
--- a/cmds/incidentd/tests/FdBuffer_test.cpp
+++ b/cmds/incidentd/tests/FdBuffer_test.cpp
@@ -37,6 +37,7 @@
public:
virtual void SetUp() override {
ASSERT_NE(tf.fd, -1);
+ tffd.reset(tf.fd);
ASSERT_NE(p2cPipe.init(), -1);
ASSERT_NE(c2pPipe.init(), -1);
}
@@ -56,13 +57,13 @@
EXPECT_EQ(expected[i], '\0');
}
- bool DoDataStream(int rFd, int wFd) {
+ bool DoDataStream(unique_fd* rFd, unique_fd* wFd) {
char buf[BUFFER_SIZE];
ssize_t nRead;
- while ((nRead = read(rFd, buf, BUFFER_SIZE)) > 0) {
+ while ((nRead = read(rFd->get(), buf, BUFFER_SIZE)) > 0) {
ssize_t nWritten = 0;
while (nWritten < nRead) {
- ssize_t amt = write(wFd, buf + nWritten, nRead - nWritten);
+ ssize_t amt = write(wFd->get(), buf + nWritten, nRead - nWritten);
if (amt < 0) {
return false;
}
@@ -75,6 +76,7 @@
protected:
FdBuffer buffer;
TemporaryFile tf;
+ unique_fd tffd;
Fpipe p2cPipe;
Fpipe c2pPipe;
@@ -85,7 +87,7 @@
TEST_F(FdBufferTest, ReadAndWrite) {
std::string testdata = "FdBuffer test string";
ASSERT_TRUE(WriteStringToFile(testdata, tf.path));
- ASSERT_EQ(NO_ERROR, buffer.read(tf.fd, READ_TIMEOUT));
+ ASSERT_EQ(NO_ERROR, buffer.read(&tffd, READ_TIMEOUT));
AssertBufferReadSuccessful(testdata.size());
AssertBufferContent(testdata.c_str());
}
@@ -98,7 +100,7 @@
TEST_F(FdBufferTest, ReadAndIterate) {
std::string testdata = "FdBuffer test string";
ASSERT_TRUE(WriteStringToFile(testdata, tf.path));
- ASSERT_EQ(NO_ERROR, buffer.read(tf.fd, READ_TIMEOUT));
+ ASSERT_EQ(NO_ERROR, buffer.read(&tffd, READ_TIMEOUT));
int i = 0;
EncodedBuffer::iterator it = buffer.data();
@@ -117,16 +119,16 @@
ASSERT_TRUE(pid != -1);
if (pid == 0) {
- close(c2pPipe.readFd());
+ c2pPipe.readFd().reset();
while (true) {
write(c2pPipe.writeFd(), "poo", 3);
sleep(1);
}
_exit(EXIT_FAILURE);
} else {
- close(c2pPipe.writeFd());
+ c2pPipe.writeFd().reset();
- status_t status = buffer.read(c2pPipe.readFd(), QUICK_TIMEOUT_MS);
+ status_t status = buffer.read(&c2pPipe.readFd(), QUICK_TIMEOUT_MS);
ASSERT_EQ(NO_ERROR, status);
EXPECT_TRUE(buffer.timedOut());
@@ -143,20 +145,20 @@
ASSERT_TRUE(pid != -1);
if (pid == 0) {
- close(p2cPipe.writeFd());
- close(c2pPipe.readFd());
+ p2cPipe.writeFd().reset();
+ c2pPipe.readFd().reset();
ASSERT_TRUE(WriteStringToFd(HEAD, c2pPipe.writeFd()));
- ASSERT_TRUE(DoDataStream(p2cPipe.readFd(), c2pPipe.writeFd()));
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ ASSERT_TRUE(DoDataStream(&p2cPipe.readFd(), &c2pPipe.writeFd()));
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
// Must exit here otherwise the child process will continue executing the test binary.
_exit(EXIT_SUCCESS);
} else {
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
- ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(tf.fd, p2cPipe.writeFd(),
- c2pPipe.readFd(), READ_TIMEOUT));
+ ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(&tffd, &p2cPipe.writeFd(),
+ &c2pPipe.readFd(), READ_TIMEOUT));
AssertBufferReadSuccessful(HEAD.size() + testdata.size());
AssertBufferContent(expected.c_str());
wait(&pid);
@@ -172,23 +174,23 @@
ASSERT_TRUE(pid != -1);
if (pid == 0) {
- close(p2cPipe.writeFd());
- close(c2pPipe.readFd());
+ p2cPipe.writeFd().reset();
+ c2pPipe.readFd().reset();
std::string data;
// wait for read finishes then write.
ASSERT_TRUE(ReadFdToString(p2cPipe.readFd(), &data));
data = HEAD + data;
ASSERT_TRUE(WriteStringToFd(data, c2pPipe.writeFd()));
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
// Must exit here otherwise the child process will continue executing the test binary.
_exit(EXIT_SUCCESS);
} else {
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
- ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(tf.fd, p2cPipe.writeFd(),
- c2pPipe.readFd(), READ_TIMEOUT));
+ ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(&tffd, &p2cPipe.writeFd(),
+ &c2pPipe.readFd(), READ_TIMEOUT));
AssertBufferReadSuccessful(HEAD.size() + testdata.size());
AssertBufferContent(expected.c_str());
wait(&pid);
@@ -202,18 +204,18 @@
ASSERT_TRUE(pid != -1);
if (pid == 0) {
- close(p2cPipe.writeFd());
- close(c2pPipe.readFd());
- ASSERT_TRUE(DoDataStream(p2cPipe.readFd(), c2pPipe.writeFd()));
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ p2cPipe.writeFd().reset();
+ c2pPipe.readFd().reset();
+ ASSERT_TRUE(DoDataStream(&p2cPipe.readFd(), &c2pPipe.writeFd()));
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
_exit(EXIT_SUCCESS);
} else {
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
- ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(tf.fd, p2cPipe.writeFd(),
- c2pPipe.readFd(), READ_TIMEOUT));
+ ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(&tffd, &p2cPipe.writeFd(),
+ &c2pPipe.readFd(), READ_TIMEOUT));
AssertBufferReadSuccessful(0);
AssertBufferContent("");
wait(&pid);
@@ -223,24 +225,24 @@
TEST_F(FdBufferTest, ReadInStreamMoreThan4MB) {
const std::string testFile = kTestDataPath + "morethan4MB.txt";
size_t fourMB = (size_t)4 * 1024 * 1024;
- int fd = open(testFile.c_str(), O_RDONLY | O_CLOEXEC);
- ASSERT_NE(fd, -1);
+ unique_fd fd(open(testFile.c_str(), O_RDONLY | O_CLOEXEC));
+ ASSERT_NE(fd.get(), -1);
int pid = fork();
ASSERT_TRUE(pid != -1);
if (pid == 0) {
- close(p2cPipe.writeFd());
- close(c2pPipe.readFd());
- ASSERT_TRUE(DoDataStream(p2cPipe.readFd(), c2pPipe.writeFd()));
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ p2cPipe.writeFd().reset();
+ c2pPipe.readFd().reset();
+ ASSERT_TRUE(DoDataStream(&p2cPipe.readFd(), &c2pPipe.writeFd()));
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
_exit(EXIT_SUCCESS);
} else {
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
- ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(fd, p2cPipe.writeFd(),
- c2pPipe.readFd(), READ_TIMEOUT));
+ ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(&fd, &p2cPipe.writeFd(),
+ &c2pPipe.readFd(), READ_TIMEOUT));
EXPECT_EQ(buffer.size(), fourMB);
EXPECT_FALSE(buffer.timedOut());
EXPECT_TRUE(buffer.truncated());
@@ -266,18 +268,18 @@
ASSERT_TRUE(pid != -1);
if (pid == 0) {
- close(p2cPipe.writeFd());
- close(c2pPipe.readFd());
+ p2cPipe.writeFd().reset();
+ c2pPipe.readFd().reset();
while (true) {
sleep(1);
}
_exit(EXIT_FAILURE);
} else {
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
+ p2cPipe.readFd().reset();
+ c2pPipe.writeFd().reset();
- ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(tf.fd, p2cPipe.writeFd(),
- c2pPipe.readFd(), QUICK_TIMEOUT_MS));
+ ASSERT_EQ(NO_ERROR, buffer.readProcessedDataInStream(&tffd, &p2cPipe.writeFd(),
+ &c2pPipe.readFd(), QUICK_TIMEOUT_MS));
EXPECT_TRUE(buffer.timedOut());
kill(pid, SIGKILL); // reap the child process
}
diff --git a/cmds/incidentd/tests/PrivacyBuffer_test.cpp b/cmds/incidentd/tests/PrivacyBuffer_test.cpp
index c7c69a7..5edc0c7 100644
--- a/cmds/incidentd/tests/PrivacyBuffer_test.cpp
+++ b/cmds/incidentd/tests/PrivacyBuffer_test.cpp
@@ -58,7 +58,8 @@
void writeToFdBuffer(string str) {
ASSERT_TRUE(WriteStringToFile(str, tf.path));
- ASSERT_EQ(NO_ERROR, buffer.read(tf.fd, 10000));
+ unique_fd tffd(tf.fd);
+ ASSERT_EQ(NO_ERROR, buffer.read(&tffd, 10000));
ASSERT_EQ(str.size(), buffer.size());
}