Solve two potential problems with one shot.

First; we absolutely do NOT want to waste our time creating a pipe,
when the caller has their own file descriptors all set up to give to
the child process (and use itself).  We can also presume a single
ended pipe is about as interesting as the sound of one hand clapping.

Create the pipe only when we don't already have any child/parent pipes
set up, and when the caller passes no files for us to use.  Otherwise,
we simply dup for our own use rather than dup2.

Second; we absolutely cannot dup2 into the static 'no_file' special fd,
so we'll guard against this and also dup, instead, for this case.



git-svn-id: http://svn.apache.org/repos/asf/apr/apr/trunk@584500 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 66 insertions, 77 deletions

diff --git a/threadproc/unix/proc.c b/threadproc/unix/proc.c
index 21b44d4b8..7ade722fa 100644
--- a/threadproc/unix/proc.c
+++ b/threadproc/unix/proc.c
@@ -44,78 +44,41 @@ APR_DECLARE(apr_status_t) apr_procattr_io_set(apr_procattr_t *attr,
                                               apr_int32_t out,
                                               apr_int32_t err)
 {
-    apr_status_t status;
-    if ((in != APR_NO_PIPE) && (in != APR_NO_FILE)) {
-        if ((status = apr_file_pipe_create(&attr->child_in, &attr->parent_in,
-                                           attr->pool)) != APR_SUCCESS) {
-            return status;
-        }
+    apr_status_t rv;
 
-        switch (in) {
-        case APR_FULL_BLOCK:
-            break;
-        case APR_PARENT_BLOCK:
-            apr_file_pipe_timeout_set(attr->child_in, 0);
-            break;
-        case APR_CHILD_BLOCK:
-            apr_file_pipe_timeout_set(attr->parent_in, 0);
-            break;
-        default:
-            apr_file_pipe_timeout_set(attr->child_in, 0);
-            apr_file_pipe_timeout_set(attr->parent_in, 0);
-        }
+    if ((in != APR_NO_PIPE) && (in != APR_NO_FILE)) {
+        /* APR_CHILD_BLOCK maps to APR_WRITE_BLOCK, while
+         * APR_PARENT_BLOCK maps to APR_READ_BLOCK, so transpose 
+         * the CHILD/PARENT blocking flags for the stdin pipe.
+         * stdout/stderr map to the correct mode by default.
+         */
+        if (in == APR_CHILD_BLOCK)
+            in = APR_READ_BLOCK;
+        else if (in == APR_PARENT_BLOCK)
+            in = APR_WRITE_BLOCK;
+
+        if ((rv = apr_file_pipe_create_ex(&attr->child_in, &attr->parent_in,
+                                          in, attr->pool)) != APR_SUCCESS)
+            return rv;
     }
-    else if (in == APR_NO_FILE) {
+    else if (in == APR_NO_FILE)
         attr->child_in = &no_file;
-    }
 
     if ((out != APR_NO_PIPE) && (out != APR_NO_FILE)) {
-        if ((status = apr_file_pipe_create(&attr->parent_out, &attr->child_out,
-                                           attr->pool)) != APR_SUCCESS) {
-            return status;
-        }
-
-        switch (out) {
-        case APR_FULL_BLOCK:
-            break;
-        case APR_PARENT_BLOCK:
-            apr_file_pipe_timeout_set(attr->child_out, 0);
-            break;
-        case APR_CHILD_BLOCK:
-            apr_file_pipe_timeout_set(attr->parent_out, 0);
-            break;
-        default:
-            apr_file_pipe_timeout_set(attr->child_out, 0);
-            apr_file_pipe_timeout_set(attr->parent_out, 0);
-        }
+        if ((rv = apr_file_pipe_create_ex(&attr->parent_out, &attr->child_out,
+                                          out, attr->pool)) != APR_SUCCESS)
+            return rv;
     }
-    else if (out == APR_NO_FILE) {
+    else if (out == APR_NO_FILE)
         attr->child_out = &no_file;
-    }
 
     if ((err != APR_NO_PIPE) && (err != APR_NO_FILE)) {
-        if ((status = apr_file_pipe_create(&attr->parent_err, &attr->child_err,
-                                           attr->pool)) != APR_SUCCESS) {
-            return status;
-        }
-
-        switch (err) {
-        case APR_FULL_BLOCK:
-            break;
-        case APR_PARENT_BLOCK:
-            apr_file_pipe_timeout_set(attr->child_err, 0);
-            break;
-        case APR_CHILD_BLOCK:
-            apr_file_pipe_timeout_set(attr->parent_err, 0);
-            break;
-        default:
-            apr_file_pipe_timeout_set(attr->child_err, 0);
-            apr_file_pipe_timeout_set(attr->parent_err, 0);
-        }
+        if ((rv = apr_file_pipe_create_ex(&attr->parent_err, &attr->child_err,
+                                          err, attr->pool)) != APR_SUCCESS)
+            return rv;
     }
-    else if (err == APR_NO_FILE) {
+    else if (err == APR_NO_FILE)
         attr->child_err = &no_file;
-    }
 
     return APR_SUCCESS;
 }
@@ -127,14 +90,23 @@ APR_DECLARE(apr_status_t) apr_procattr_child_in_set(apr_procattr_t *attr,
 {
     apr_status_t rv = APR_SUCCESS;
 
-    if (attr->child_in == NULL && attr->parent_in == NULL)
+    if (attr->child_in == NULL && attr->parent_in == NULL
+           && child_in == NULL && parent_in == NULL)
         rv = apr_file_pipe_create(&attr->child_in, &attr->parent_in, attr->pool);
     
-    if (child_in != NULL && rv == APR_SUCCESS)
-        rv = apr_file_dup2(attr->child_in, child_in, attr->pool);
+    if (child_in != NULL && rv == APR_SUCCESS) {
+        if (attr->child_in && (attr->child_in->filedes != -1))
+            rv = apr_file_dup2(attr->child_in, child_in, attr->pool);
+        else
+            rv = apr_file_dup(&attr->child_in, child_in, attr->pool);
+    }
 
-    if (parent_in != NULL && rv == APR_SUCCESS)
-        rv = apr_file_dup2(attr->parent_in, parent_in, attr->pool);
+    if (parent_in != NULL && rv == APR_SUCCESS) {
+        if (attr->parent_in)
+            rv = apr_file_dup2(attr->parent_in, parent_in, attr->pool);
+        else
+            rv = apr_file_dup(&attr->parent_in, parent_in, attr->pool);
+    }
 
     return rv;
 }
@@ -146,14 +118,23 @@ APR_DECLARE(apr_status_t) apr_procattr_child_out_set(apr_procattr_t *attr,
 {
     apr_status_t rv = APR_SUCCESS;
 
-    if (attr->child_out == NULL && attr->parent_out == NULL)
+    if (attr->child_out == NULL && attr->parent_out == NULL
+           && child_out == NULL && parent_out == NULL)
         rv = apr_file_pipe_create(&attr->child_out, &attr->parent_out, attr->pool);
 
-    if (child_out != NULL && rv == APR_SUCCESS)
-        rv = apr_file_dup2(attr->child_out, child_out, attr->pool);
+    if (child_out != NULL && rv == APR_SUCCESS) {
+        if (attr->child_out && (attr->child_out->filedes != -1))
+            rv = apr_file_dup2(attr->child_out, child_out, attr->pool);
+        else
+            rv = apr_file_dup(&attr->child_out, child_out, attr->pool);
+    }
 
-    if (parent_out != NULL && rv == APR_SUCCESS)
-        rv = apr_file_dup2(attr->parent_out, parent_out, attr->pool);
+    if (parent_out != NULL && rv == APR_SUCCESS) {
+        if (attr->parent_out)
+            rv = apr_file_dup2(attr->parent_out, parent_out, attr->pool);
+        else
+            rv = apr_file_dup(&attr->parent_out, parent_out, attr->pool);
+    }
 
     return rv;
 }
@@ -165,14 +146,22 @@ APR_DECLARE(apr_status_t) apr_procattr_child_err_set(apr_procattr_t *attr,
 {
     apr_status_t rv = APR_SUCCESS;
 
-    if (attr->child_err == NULL && attr->parent_err == NULL)
+    if (attr->child_err == NULL && attr->parent_err == NULL
+           && child_err == NULL && parent_err == NULL)
         rv = apr_file_pipe_create(&attr->child_err, &attr->parent_err, attr->pool);
 
-    if (child_err != NULL && rv == APR_SUCCESS)
-        rv = apr_file_dup2(attr->child_err, child_err, attr->pool);
-
-    if (parent_err != NULL && rv == APR_SUCCESS)
-        rv = apr_file_dup2(attr->parent_err, parent_err, attr->pool);
+    if (child_err != NULL && rv == APR_SUCCESS) {
+        if (attr->child_err && (attr->child_err->filedes != -1))
+            rv = apr_file_dup2(attr->child_err, child_err, attr->pool);
+        else
+            rv = apr_file_dup(&attr->child_err, child_err, attr->pool);
+    }
+    if (parent_err != NULL && rv == APR_SUCCESS) {
+        if (attr->parent_err)
+            rv = apr_file_dup2(attr->parent_err, parent_err, attr->pool);
+        else
+            rv = apr_file_dup(&attr->parent_err, parent_err, attr->pool);
+    }
 
     return rv;
 }