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
