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