popen() requires check for fdopen() failure

Move fdopen() up near other resource allocation like malloc(); do proper
deallocation on failure later on in the function.

Submitted by:	Ramachandra Topannavar <rtopannavar@panasas.com>
Reviewed by:	jilles
Approved by:	jhb (mentor)
MFC after:	2 weeks
Sponsored by:	Panasas, Inc.
Differential Revision:	https://reviews.freebsd.org/D4126

M    lib/libc/gen/popen.c

1 files changed, 24 insertions, 10 deletions

diff --git a/lib/libc/gen/popen.c b/lib/libc/gen/popen.c
index a7a9708..e469ab6 100644
--- a/lib/libc/gen/popen.c
+++ b/lib/libc/gen/popen.c
@@ -72,6 +72,7 @@ popen(const char *command, const char *type)
 	struct pid *cur;
 	FILE *iop;
 	int pdes[2], pid, twoway, cloexec;
+	int pdes_unused_in_parent;
 	char *argv[4];
 	struct pid *p;
 
@@ -98,6 +99,20 @@ popen(const char *command, const char *type)
 		return (NULL);
 	}
 
+	if (*type == 'r') {
+		iop = fdopen(pdes[0], type);
+		pdes_unused_in_parent = pdes[1];
+	} else {
+		iop = fdopen(pdes[1], type);
+		pdes_unused_in_parent = pdes[0];
+	}
+	if (iop == NULL) {
+		(void)_close(pdes[0]);
+		(void)_close(pdes[1]);
+		free(cur);
+		return (NULL);
+	}
+
 	argv[0] = "sh";
 	argv[1] = "-c";
 	argv[2] = (char *)command;
@@ -107,9 +122,14 @@ popen(const char *command, const char *type)
 	switch (pid = vfork()) {
 	case -1:			/* Error. */
 		THREAD_UNLOCK();
-		(void)_close(pdes[0]);
-		(void)_close(pdes[1]);
+		/*
+		 * The _close() closes the unused end of pdes[], while
+		 * the fclose() closes the used end of pdes[], *and* cleans
+		 * up iop.
+		 */
+		(void)_close(pdes_unused_in_parent);
 		free(cur);
+		(void)fclose(iop);
 		return (NULL);
 		/* NOTREACHED */
 	case 0:				/* Child. */
@@ -145,14 +165,8 @@ popen(const char *command, const char *type)
 	}
 	THREAD_UNLOCK();
 
-	/* Parent; assume fdopen can't fail. */
-	if (*type == 'r') {
-		iop = fdopen(pdes[0], type);
-		(void)_close(pdes[1]);
-	} else {
-		iop = fdopen(pdes[1], type);
-		(void)_close(pdes[0]);
-	}
+	/* Parent. */
+	(void)_close(pdes_unused_in_parent);
 
 	/* Link into list of file descriptors. */
 	cur->fp = iop;