svn commit: r227329 - in /httpd/httpd/branches/2.0.x: CHANGES STATUS server/log.c

[jo...@apache.org]

Author: jorton
Date: Thu Aug  4 02:35:38 2005
New Revision: 227329

URL: http://svn.apache.org/viewcvs?rev=227329&view=rev
Log:
Merge r170441, r170573, r170719 from trunk:

* server/log.c (piped_log_spawn): Return the APR error code,
as expected by piped_log_maintenance (and fixing "Unknown
error" messages in error path there).
(ap_open_piped_log): Expect an APR error code.

* server/log.c (ap_open_piped_log): Remove errno handling.

* server/log.c (piped_log_spawn): Don't leak an fd for each
invocation.

Reviewed by: jorton, trawick, wrowe

Modified:
    httpd/httpd/branches/2.0.x/CHANGES
    httpd/httpd/branches/2.0.x/STATUS
    httpd/httpd/branches/2.0.x/server/log.c

Modified: httpd/httpd/branches/2.0.x/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/CHANGES?rev=227329&r1=227328&r2=227329&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.0.x/CHANGES [utf-8] Thu Aug  4 02:35:38 2005
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.0.55
 
+  *) Fix a file descriptor leak when starting piped loggers.  [Joe Orton]
+
   *) mod_ldap: Avoid segfaults when opening connections if using a version
      of OpenLDAP older than 2.2.21.  [Brad Nicholes]
 

Modified: httpd/httpd/branches/2.0.x/STATUS
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/STATUS?rev=227329&r1=227328&r2=227329&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/STATUS (original)
+++ httpd/httpd/branches/2.0.x/STATUS Thu Aug  4 02:35:38 2005
@@ -199,16 +199,6 @@
 
        +1: jim, pquerna, wrowe
 
-    *) Fix fd leak in piped logging code, fix error handling, and remove
-       dead errno handling.
-         http://svn.apache.org/viewcvs?rev=170441&view=rev
-         http://svn.apache.org/viewcvs?rev=170537&view=rev
-         http://svn.apache.org/viewcvs?rev=170719&view=rev
-       all-in-one patch incremental to the PR 26467 fix:
-         http://people.apache.org/~jorton/ap_pipedlog2.diff
-       +1: jorton, trawick, wrowe
-       [yes, I will write a CHANGES entry too]
-
     *) several changes to improve logging of connection-oriented errors, including
        ap_log_cerror() API (needs minor bump in addition to changes below)
          http://cvs.apache.org/viewcvs.cgi/httpd-2.0/server/core.c?r1=1.289&r2=1.291

Modified: httpd/httpd/branches/2.0.x/server/log.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/server/log.c?rev=227329&r1=227328&r2=227329&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/server/log.c (original)
+++ httpd/httpd/branches/2.0.x/server/log.c Thu Aug  4 02:35:38 2005
@@ -763,9 +763,9 @@
 /* forward declaration */
 static void piped_log_maintenance(int reason, void *data, apr_wait_t status);
 
-static int piped_log_spawn(piped_log *pl)
+/* Spawn the piped logger process pl->program. */
+static apr_status_t piped_log_spawn(piped_log *pl)
 {
-    int rc = 0;
     apr_procattr_t *procattr;
     apr_proc_t *procnew = NULL;
     apr_status_t status;
@@ -783,7 +783,6 @@
         ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL,
                      "piped_log_spawn: unable to setup child process '%s': %s",
                      pl->program, apr_strerror(status, buf, sizeof(buf)));
-        rc = -1;
     }
     else {
         char **args;
@@ -797,7 +796,11 @@
 
         if (status == APR_SUCCESS) {
             pl->pid = procnew;
-            ap_piped_log_write_fd(pl) = procnew->in;
+            /* procnew->in was dup2'd from ap_piped_log_write_fd(pl);
+             * since the original fd is still valid, close the copy to
+             * avoid a leak. */
+            apr_file_close(procnew->in);
+            procnew->in = NULL;
             apr_proc_other_child_register(procnew, piped_log_maintenance, pl,
                                           ap_piped_log_write_fd(pl), pl->p);
             close_handle_in_child(pl->p, ap_piped_log_read_fd(pl));
@@ -808,11 +811,10 @@
             ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL,
                          "unable to start piped log program '%s': %s",
                          pl->program, apr_strerror(status, buf, sizeof(buf)));
-            rc = -1;
         }
     }
 
-    return rc;
+    return status;
 }
 
 
@@ -904,12 +906,10 @@
     }
     apr_pool_cleanup_register(p, pl, piped_log_cleanup,
                               piped_log_cleanup_for_exec);
-    if (piped_log_spawn(pl) == -1) {
-        int save_errno = errno;
+    if (piped_log_spawn(pl) != APR_SUCCESS) {
         apr_pool_cleanup_kill(p, pl, piped_log_cleanup);
         apr_file_close(ap_piped_log_read_fd(pl));
         apr_file_close(ap_piped_log_write_fd(pl));
-        errno = save_errno;
         return NULL;
     }
     return pl;