httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: svn commit: r735516 - /httpd/httpd/trunk/server/log.c
Date Thu, 14 May 2009 13:49:40 GMT
On Thu, May 14, 2009 at 12:51:18PM +0200, Rainer Jung wrote:
> On 13.05.2009 22:38, William A. Rowe, Jr. wrote:
> > Please revert the introduction of a _wrapper struct and let's simply
> > fix the piped_log structure?
> 
> Do we really want to add it to the public API?

There's no need for that structure to be exposed at all.  I had a patch 
to fix that lying around - updated for trunk below and can commit later 
when I've had time to test.

Index: server/log.c
===================================================================
--- server/log.c	(revision 774761)
+++ server/log.c	(working copy)
@@ -149,6 +149,37 @@
 
 static read_handle_t *read_handles;
 
+/**
+ * @brief The piped logging structure.  
+ *
+ * Piped logs are used to move functionality out of the main server.  
+ * For example, log rotation is done with piped logs.
+ */
+struct piped_log {
+    /** The pool to use for the piped log */
+    apr_pool_t *p;
+    /** The pipe between the server and the logging process */
+    apr_file_t *read_fd, *write_fd;
+    /* XXX - an #ifdef that needs to be eliminated from public view. Shouldn't
+     * be hard */
+#ifdef AP_HAVE_RELIABLE_PIPED_LOGS
+    /** The name of the program the logging process is running */
+    char *program;
+    /** The pid of the logging process */
+    apr_proc_t *pid;
+#endif
+};
+
+apr_file_t *ap_piped_log_read_fd(piped_log *pl)
+{
+    return pl->read_fd;
+}
+
+apr_file_t *ap_piped_log_write_fd(piped_log *pl)
+{
+    return pl->write_fd;
+}
+
 /* clear_handle_list() is called when plog is cleared; at that
  * point we need to forget about our old list of pipe read
  * handles.  We let the plog cleanups close the actual pipes.
@@ -876,8 +907,8 @@
         ((status = apr_procattr_cmdtype_set(procattr,
                                             APR_SHELLCMD_ENV)) != APR_SUCCESS) ||
         ((status = apr_procattr_child_in_set(procattr,
-                                             ap_piped_log_read_fd(pl),
-                                             ap_piped_log_write_fd(pl)))
+                                             pl->read_fd,
+                                             pl->write_fd))
         != APR_SUCCESS) ||
         ((status = apr_procattr_child_errfn_set(procattr, log_child_errfn))
          != APR_SUCCESS) ||
@@ -900,14 +931,14 @@
 
         if (status == APR_SUCCESS) {
             pl->pid = procnew;
-            /* procnew->in was dup2'd from ap_piped_log_write_fd(pl);
+            /* procnew->in was dup2'd from pl->write_fd;
              * 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));
+                                          pl->write_fd, pl->p);
+            close_handle_in_child(pl->p, pl->read_fd);
         }
         else {
             char buf[120];
@@ -979,8 +1010,8 @@
 {
     piped_log *pl = data;
 
-    apr_file_close(ap_piped_log_read_fd(pl));
-    apr_file_close(ap_piped_log_write_fd(pl));
+    apr_file_close(pl->read_fd);
+    apr_file_close(pl->write_fd);
     return APR_SUCCESS;
 }
 
@@ -1004,8 +1035,8 @@
     pl->p = p;
     pl->program = apr_pstrdup(p, program);
     pl->pid = NULL;
-    if (apr_file_pipe_create_ex(&ap_piped_log_read_fd(pl),
-                                &ap_piped_log_write_fd(pl),
+    if (apr_file_pipe_create_ex(&pl->read_fd,
+                                &pl->write_fd,
                                 APR_FULL_BLOCK, p) != APR_SUCCESS) {
         return NULL;
     }
@@ -1013,8 +1044,8 @@
                               piped_log_cleanup_for_exec);
     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));
+        apr_file_close(pl->read_fd);
+        apr_file_close(pl->write_fd);
         return NULL;
     }
     return pl;
@@ -1026,7 +1057,7 @@
 {
     piped_log *pl = data;
 
-    apr_file_close(ap_piped_log_write_fd(pl));
+    apr_file_close(pl->write_fd);
     return APR_SUCCESS;
 }
 
@@ -1046,8 +1077,8 @@
 
     pl = apr_palloc(p, sizeof (*pl));
     pl->p = p;
-    ap_piped_log_read_fd(pl) = NULL;
-    ap_piped_log_write_fd(pl) = dummy;
+    pl->read_fd = NULL;
+    pl->write_fd = dummy;
     apr_pool_cleanup_register(p, pl, piped_log_cleanup, piped_log_cleanup);
 
     return pl;
Index: include/http_log.h
===================================================================
--- include/http_log.h	(revision 774761)
+++ include/http_log.h	(working copy)
@@ -268,27 +268,6 @@
 typedef struct piped_log piped_log;
 
 /**
- * @brief The piped logging structure.  
- *
- * Piped logs are used to move functionality out of the main server.  
- * For example, log rotation is done with piped logs.
- */
-struct piped_log {
-    /** The pool to use for the piped log */
-    apr_pool_t *p;
-    /** The pipe between the server and the logging process */
-    apr_file_t *fds[2];
-    /* XXX - an #ifdef that needs to be eliminated from public view. Shouldn't
-     * be hard */
-#ifdef AP_HAVE_RELIABLE_PIPED_LOGS
-    /** The name of the program the logging process is running */
-    char *program;
-    /** The pid of the logging process */
-    apr_proc_t *pid;
-#endif
-};
-
-/**
  * Open the piped log process
  * @param p The pool to allocate out of
  * @param program The program to run in the logging process
@@ -303,18 +282,18 @@
 AP_DECLARE(void) ap_close_piped_log(piped_log *pl);
 
 /**
- * A macro to access the read side of the piped log pipe
+ * A function to return the read side of the piped log pipe
  * @param pl The piped log structure
  * @return The native file descriptor
  */
-#define ap_piped_log_read_fd(pl)	((pl)->fds[0])
+AP_DECLARE(apr_file_t *) ap_piped_log_read_fd(piped_log *pl);
 
 /**
- * A macro to access the write side of the piped log pipe
+ * A function to return the write side of the piped log pipe
  * @param pl The piped log structure
  * @return The native file descriptor
  */
-#define ap_piped_log_write_fd(pl)	((pl)->fds[1])
+AP_DECLARE(apr_file_t *) ap_piped_log_write_fd(piped_log *pl);
 
 /**
  * hook method to log error messages 


Mime
View raw message