From f9c534df01aef4bc3d61b6bca0c19d4d42239e7f Mon Sep 17 00:00:00 2001
From: Dan Goodliffe <dan@randomdan.homeip.net>
Date: Sat, 12 Sep 2015 18:03:33 +0100
Subject: Tidy up process pipes error handling with new scope exit
functionality and move closing handles into a helper
---
libadhocutil/processPipes.cpp | 70 ++++++++++++++++++++-----------------------
libadhocutil/processPipes.h | 3 ++
2 files changed, 36 insertions(+), 37 deletions(-)
diff --git a/libadhocutil/processPipes.cpp b/libadhocutil/processPipes.cpp
index 229c9d2..511f415 100644
--- a/libadhocutil/processPipes.cpp
+++ b/libadhocutil/processPipes.cpp
@@ -4,6 +4,7 @@
#include <string.h>
#include <sys/resource.h>
#include <stdexcept>
+#include "scopeExit.h"
namespace AdHoc {
namespace System {
@@ -14,47 +15,36 @@ ProcessPipes::ProcessPipes(const std::vector<std::string> & args, bool i, bool o
error(-1)
{
int ipipes[2], opipes[2], epipes[2];
+ ScopeExit tidyUp;
if (i) {
if (pipe(ipipes)) {
throw std::runtime_error("Failed to create stdin pipe");
}
+ tidyUp.onFailure.push_back([ipipes] {
+ close(ipipes[0]);
+ close(ipipes[1]);
+ });
}
if (o) {
if (pipe(opipes)) {
- if (i) {
- close(ipipes[0]);
- close(ipipes[1]);
- }
throw std::runtime_error("Failed to create stdout pipe");
}
+ tidyUp.onFailure.push_back([opipes] {
+ close(opipes[0]);
+ close(opipes[1]);
+ });
}
if (e) {
if (pipe(epipes)) {
- if (i) {
- close(ipipes[0]);
- close(ipipes[1]);
- }
- if (o) {
- close(opipes[0]);
- close(opipes[1]);
- }
throw std::runtime_error("Failed to create stderr pipe");
}
+ tidyUp.onFailure.push_back([epipes] {
+ close(epipes[0]);
+ close(epipes[1]);
+ });
}
switch (child = fork()) {
case -1: // fail
- if (i) {
- close(ipipes[0]);
- close(ipipes[1]);
- }
- if (o) {
- close(opipes[0]);
- close(opipes[1]);
- }
- if (e) {
- close(epipes[0]);
- close(epipes[1]);
- }
throw std::runtime_error("Failed to fork");
default: // parent
if (i) {
@@ -71,8 +61,6 @@ ProcessPipes::ProcessPipes(const std::vector<std::string> & args, bool i, bool o
}
break;
case 0: // in child
- rlimit lim;
- getrlimit(RLIMIT_NOFILE, &lim);
if (i) {
close(ipipes[1]);
dup2(ipipes[0], 0);
@@ -85,17 +73,7 @@ ProcessPipes::ProcessPipes(const std::vector<std::string> & args, bool i, bool o
close(epipes[0]);
dup2(epipes[1], 2);
}
- std::vector<struct pollfd> fds;
- fds.reserve(lim.rlim_max);
- for (int n = 3; n < (int)lim.rlim_max; n += 1) {
- fds.push_back({n, 0, 0});
- }
- poll(&fds.front(), fds.size(), 0);
- for(const auto & pfd : fds) {
- if (!(pfd.revents & POLLNVAL)) {
- close(pfd.fd);
- }
- }
+ closeAllOpenFiles();
char * buf[100];
char ** w = &buf[0];
for (const auto & p : args) {
@@ -139,6 +117,24 @@ ProcessPipes::pid() const
return child;
}
+void
+ProcessPipes::closeAllOpenFiles()
+{
+ rlimit lim;
+ getrlimit(RLIMIT_NOFILE, &lim);
+ std::vector<struct pollfd> fds;
+ fds.reserve(lim.rlim_max);
+ for (int n = 3; n < (int)lim.rlim_max; n += 1) {
+ fds.push_back({n, 0, 0});
+ }
+ poll(&fds.front(), fds.size(), 0);
+ for(const auto & pfd : fds) {
+ if (!(pfd.revents & POLLNVAL)) {
+ close(pfd.fd);
+ }
+ }
+}
+
}
}
diff --git a/libadhocutil/processPipes.h b/libadhocutil/processPipes.h
index 195d8e9..bbf7508 100644
--- a/libadhocutil/processPipes.h
+++ b/libadhocutil/processPipes.h
@@ -31,6 +31,9 @@ class DLL_PUBLIC ProcessPipes {
/** Process id of child. */
pid_t pid() const;
+ /** Close all open file handles as determined by rlimit and poll. */
+ static void closeAllOpenFiles();
+
private:
ProcessPipes(const ProcessPipes &) = delete;
void operator=(const ProcessPipes &) = delete;
--
cgit v1.2.3