You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Michael Douglass <mi...@staff.texas.net> on 1999/07/30 19:50:11 UTC

[patch included] Re: use of stdio for error_log

Here's the modifications I made to apache 1.3.3 (sorry I didn't grab the
latest--if there are any changes between 1.3.3 and 1.3.6 I'm certain
you guys can figure them out).  I also don't know if you really want to
merge this into the core as there are probably modules out there that
try to directly access the s->error_log variable (ie. mod_perl--though
mod_perl's use is very limited and not hard to fix at all).

Here goes; created using gnu diff:

diff -rc apache_1.3.3/src/include/httpd.h ../apache_1.3.3/src/include/httpd.h
*** apache_1.3.3/src/include/httpd.h    Wed Oct  7 04:19:06 1998
--- ../apache_1.3.3/src/include/httpd.h Fri Jul 30 11:04:14 1999
***************
*** 842,848 ****
      /* Log files --- note that transfer log is now in the modules... */

      char *error_fname;
!     FILE *error_log;
      int loglevel;

      /* Module-specific configuration for server, and defaults... */
--- 842,848 ----
      /* Log files --- note that transfer log is now in the modules... */

      char *error_fname;
!     int error_log;
      int loglevel;

      /* Module-specific configuration for server, and defaults... */
diff -rc apache_1.3.3/src/main/http_config.c ../apache_1.3.3/src/main/http_config.c
*** apache_1.3.3/src/main/http_config.c Fri Sep 25 19:07:08 1998
--- ../apache_1.3.3/src/main/http_config.c      Fri Jul 30 11:51:36 1999
***************
*** 1408,1414 ****
      s->server_admin = DEFAULT_ADMIN;
      s->server_hostname = NULL;
      s->error_fname = DEFAULT_ERRORLOG;
!     s->error_log = stderr;
      s->loglevel = DEFAULT_LOGLEVEL;
      s->srm_confname = RESOURCE_CONFIG_FILE;
      s->access_confname = ACCESS_CONFIG_FILE;
--- 1408,1414 ----
      s->server_admin = DEFAULT_ADMIN;
      s->server_hostname = NULL;
      s->error_fname = DEFAULT_ERRORLOG;
!     s->error_log = STDERR_FILENO;
      s->loglevel = DEFAULT_LOGLEVEL;
      s->srm_confname = RESOURCE_CONFIG_FILE;
      s->access_confname = ACCESS_CONFIG_FILE;
diff -rc apache_1.3.3/src/main/http_log.c ../apache_1.3.3/src/main/http_log.c
*** apache_1.3.3/src/main/http_log.c    Sat Oct  3 09:28:55 1998
--- ../apache_1.3.3/src/main/http_log.c Fri Jul 30 12:02:05 1999
***************
*** 195,201 ****
            exit(1);
        }

!       s->error_log = dummy;
      }

  #ifdef HAVE_SYSLOG
--- 195,201 ----
            exit(1);
        }

!       s->error_log = fileno(dummy);
      }

  #ifdef HAVE_SYSLOG
***************
*** 207,213 ****
            for (fac = facilities; fac->t_name; fac++) {
                if (!strcasecmp(fname, fac->t_name)) {
                    openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, fac->t_val);
!                   s->error_log = NULL;
                    return;
                }
            }
--- 207,213 ----
            for (fac = facilities; fac->t_name; fac++) {
                if (!strcasecmp(fname, fac->t_name)) {
                    openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, fac->t_val);
!                   s->error_log = -1;
                    return;
                }
            }
***************
*** 215,226 ****
        else
            openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, LOG_LOCAL7);

!       s->error_log = NULL;
      }
  #endif
      else {
        fname = ap_server_root_relative(p, s->error_fname);
!         if (!(s->error_log = ap_pfopen(p, fname, "a"))) {
              perror("fopen");
              fprintf(stderr,"httpd: could not open error log file %s.\n", fname);
              exit(1);
--- 215,233 ----
        else
            openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, LOG_LOCAL7);

!       s->error_log = -1;
      }
  #endif
      else {
+         int modeFlags = 0;
        fname = ap_server_root_relative(p, s->error_fname);
! #ifdef WIN32
!         modeFlags = _S_IREAD | _S_IWRITE;
! #else
!         modeFlags = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
! #endif
!         if ((s->error_log = ap_popenf(p, fname, O_WRONLY | O_APPEND | O_CREAT,
!                                         modeFlags)) < 0) {
              perror("fopen");
              fprintf(stderr,"httpd: could not open error log file %s.\n", fname);
              exit(1);
***************
*** 236,245 ****
      open_error_log(s_main, p);

      replace_stderr = 1;
!     if (s_main->error_log) {
        /* replace stderr with this new log */
        fflush(stderr);
!       if (dup2(fileno(s_main->error_log), STDERR_FILENO) == -1) {
            ap_log_error(APLOG_MARK, APLOG_CRIT, s_main,
                "unable to replace stderr with error_log");
        } else {
--- 243,252 ----
      open_error_log(s_main, p);

      replace_stderr = 1;
!     if (s_main->error_log >= 0) {
        /* replace stderr with this new log */
        fflush(stderr);
!       if (dup2(s_main->error_log, STDERR_FILENO) == -1) {
            ap_log_error(APLOG_MARK, APLOG_CRIT, s_main,
                "unable to replace stderr with error_log");
        } else {
***************
*** 273,281 ****
  }

  API_EXPORT(void) ap_error_log2stderr(server_rec *s) {
!     if (   s->error_log != NULL
!         && fileno(s->error_log) != STDERR_FILENO)
!         dup2(fileno(s->error_log), STDERR_FILENO);
  }

  static void log_error_core(const char *file, int line, int level,
--- 280,288 ----
  }

  API_EXPORT(void) ap_error_log2stderr(server_rec *s) {
!     if (   s->error_log >= 0
!         && s->error_log != STDERR_FILENO)
!         dup2(s->error_log, STDERR_FILENO);
  }

  static void log_error_core(const char *file, int line, int level,
***************
*** 285,291 ****
      char errstr[MAX_STRING_LEN];
      size_t len;
      int save_errno = errno;
!     FILE *logf;

      if (s == NULL) {
        /*
--- 292,298 ----
      char errstr[MAX_STRING_LEN];
      size_t len;
      int save_errno = errno;
!     int logfd;

      if (s == NULL) {
        /*
***************
*** 296,304 ****
        if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
            ((level & APLOG_LEVELMASK) > DEFAULT_LOGLEVEL))
            return;
!       logf = stderr;
      }
!     else if (s->error_log) {
        /*
         * If we are doing normal logging, don't log messages that are
         * above the server log level unless it is a startup/shutdown notice
--- 303,311 ----
        if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
            ((level & APLOG_LEVELMASK) > DEFAULT_LOGLEVEL))
            return;
!       logfd = STDERR_FILENO;
      }
!     else if (s->error_log >= 0) {
        /*
         * If we are doing normal logging, don't log messages that are
         * above the server log level unless it is a startup/shutdown notice
***************
*** 306,312 ****
        if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
            ((level & APLOG_LEVELMASK) > s->loglevel))
            return;
!       logf = s->error_log;
      }
      else {
        /*
--- 313,319 ----
        if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
            ((level & APLOG_LEVELMASK) > s->loglevel))
            return;
!       logfd = s->error_log;
      }
      else {
        /*
***************
*** 315,324 ****
         */
        if ((level & APLOG_LEVELMASK) > s->loglevel)
            return;
!       logf = NULL;
      }

!     if (logf) {
        len = ap_snprintf(errstr, sizeof(errstr), "[%s] ", ap_get_time());
      } else {
        len = 0;
--- 322,331 ----
         */
        if ((level & APLOG_LEVELMASK) > s->loglevel)
            return;
!       logfd = -1;
      }

!     if (logfd > 0) {
        len = ap_snprintf(errstr, sizeof(errstr), "[%s] ", ap_get_time());
      } else {
        len = 0;
***************
*** 413,423 ****

      len += ap_vsnprintf(errstr + len, sizeof(errstr) - len, fmt, args);

!     /* NULL if we are logging to syslog */
!     if (logf) {
!       fputs(errstr, logf);
!       fputc('\n', logf);
!       fflush(logf);
      }
  #ifdef HAVE_SYSLOG
      else {
--- 420,429 ----

      len += ap_vsnprintf(errstr + len, sizeof(errstr) - len, fmt, args);

!     /* -1 if we are logging to syslog */
!     if (logfd > 0) {
!       write(logfd, errstr, len);
!         write(logfd, "\n", 1);
      }
  #ifdef HAVE_SYSLOG
      else {
diff -rc apache_1.3.3/src/main/util_script.c ../apache_1.3.3/src/main/util_script.c
*** apache_1.3.3/src/main/util_script.c Sat Oct  3 11:49:32 1998
--- ../apache_1.3.3/src/main/util_script.c      Fri Jul 30 11:52:03 1999
***************
*** 699,705 ****
       * since that is better than allowing errors to go unnoticed.  Don't do
       * this on Win32, though, since we haven't fork()'d.
       */
!     r->server->error_log = stderr;
  #endif

  #ifdef RLIMIT_CPU
--- 699,705 ----
       * since that is better than allowing errors to go unnoticed.  Don't do
       * this on Win32, though, since we haven't fork()'d.
       */
!     r->server->error_log = fileno(stderr);
  #endif

  #ifdef RLIMIT_CPU

Re: [patch included] Re: use of stdio for error_log

Posted by Michael Douglass <mi...@staff.texas.net>.
> !       write(logfd, errstr, len);
> !         write(logfd, "\n", 1);
> 
> that will cause log entries to be mixed up when multiple processes are
> writing, you need to append the \n to errstr before writing, and call
> write() once.

Doh!  I knew that! :)

-- 
Michael Douglass
Texas Networking, Inc.

  Any sufficiently advanced bug is indistinguishable for a feature.
    -- from some indian guy

Re: [patch included] Re: use of stdio for error_log

Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
No comment about getting your patch in, because I haven't reviewed it yet.
:)

> Furthermore, since you've put it out of your mind--is there any
> hint of using actual stdio under Apache 2.0 and hence hitting bugs
> like this one again?

I don't believe we will hit this problem in 2.0.  There will be no direct
stdio calls in Apache-2.0, because we have abstracted them all out to the
APR layer.  I expect this layer to be what is used for error-logs and most
other file I/O.  Inside APR, I am not using stdio.

Ryan

_______________________________________________________________________
Ryan Bloom		rbb@raleigh.ibm.com
4205 S Miami Blvd	
RTP, NC 27709		It's a beautiful sight to see good dancers 
			doing simple steps.  It's a painful sight to
			see beginners doing complicated patterns.	


Re: [patch included] Re: use of stdio for error_log

Posted by Dean Gaudet <dg...@arctic.org>.

On Fri, 30 Jul 1999, Michael Douglass wrote:

> Furthermore, since you've put it out of your mind--is there any
> hint of using actual stdio under Apache 2.0 and hence hitting bugs
> like this one again?

when i did apache-nspr i removed all stdio, and beefed up BUFF to have the
few things needed to replace it... the BUFF in mpm is ready, but nobody
has done the work yet to ditch stdio... i'd totally welcome a patch like
that. 

one note about your patch:

!       write(logfd, errstr, len);
!         write(logfd, "\n", 1);

that will cause log entries to be mixed up when multiple processes are
writing, you need to append the \n to errstr before writing, and call
write() once.

Dean


Re: [patch included] Re: use of stdio for error_log

Posted by Michael Douglass <mi...@staff.texas.net>.
> Ohhhh... I had forgotten about good old Solaris. Actually, I think
> I blocked it out on purpose. :) 

heh; well, I unfortunately can't do that.  I run a ton of these
boxes and need my stuff to work. :)  Any chance on getting this
patch (even if you rename the variable per the other gentleman's
comment) incorporated into apache?  I don't have any problem with
patching mine each time I need to--but this is a problem that will
always be present with Solaris and Apache.

Furthermore, since you've put it out of your mind--is there any
hint of using actual stdio under Apache 2.0 and hence hitting bugs
like this one again?

-- 
Michael Douglass
Texas Networking, Inc.

  Any sufficiently advanced bug is indistinguishable for a feature.
    -- from some indian guy

Re: [patch included] Re: use of stdio for error_log

Posted by Michael Douglass <mi...@staff.texas.net>.
On Fri, Jul 30, 1999 at 02:18:31PM -0400, Jim Jagielski said:
> Michael Douglass wrote:
> > 
> > 
> > Here's the modifications I made to apache 1.3.3 (sorry I didn't grab the
> > latest--if there are any changes between 1.3.3 and 1.3.6 I'm certain
> > you guys can figure them out).  I also don't know if you really want to
> > merge this into the core as there are probably modules out there that
> > try to directly access the s->error_log variable (ie. mod_perl--though
> > mod_perl's use is very limited and not hard to fix at all).
> > 
> 
> I'm confused. Are you saying that your fopen() limit is smaller than your
> open() limit?

Yes; under Solaris there is a 256 limit in the stupid libraries for fopen.
Here's the information on the apache web page about the problem.

http://www.apache.org/docs/misc/descriptors.html

By reading this web page it sounds like the only real "big" thing
you guys use stdio for is the error_log--and with the API wrapping
most intelligent access to the error_log files I don't see any
reason to use stdio--especially since you're already doing all of
the formatting using ap_vsnprintf() and doing a simple fputs() and
fputc() call to the logfile.

-- 
Michael Douglass
Texas Networking, Inc.

  Any sufficiently advanced bug is indistinguishable for a feature.
    -- from some indian guy

Re: [patch included] Re: use of stdio for error_log

Posted by Michael Douglass <mi...@staff.texas.net>.
That is a *very* good point.

On Fri, Jul 30, 1999 at 11:04:15AM -0700, Scott Hess said:
> Instead of changing "FILE *error_log" to "int error_log", it would probably
> be safer to change the name of the element, say to "int err_log" or "int
> error_fd", or something of the sort.  That way any existing code relying on
> error_log will break in informative ways.
> 
> Later,
> scott
> 
> ----- Original Message -----
> From: Michael Douglass <mi...@staff.texas.net>
> To: <ne...@apache.org>
> Sent: Friday, July 30, 1999 10:50 AM
> Subject: [patch included] Re: use of stdio for error_log
> 
> 
> >
> > Here's the modifications I made to apache 1.3.3 (sorry I didn't grab the
> > latest--if there are any changes between 1.3.3 and 1.3.6 I'm certain
> > you guys can figure them out).  I also don't know if you really want to
> > merge this into the core as there are probably modules out there that
> > try to directly access the s->error_log variable (ie. mod_perl--though
> > mod_perl's use is very limited and not hard to fix at all).
> >
> > Here goes; created using gnu diff:
> >
> > diff -rc apache_1.3.3/src/include/httpd.h
> ../apache_1.3.3/src/include/httpd.h
> > *** apache_1.3.3/src/include/httpd.h    Wed Oct  7 04:19:06 1998
> > --- ../apache_1.3.3/src/include/httpd.h Fri Jul 30 11:04:14 1999
> > ***************
> > *** 842,848 ****
> >       /* Log files --- note that transfer log is now in the modules... */
> >
> >       char *error_fname;
> > !     FILE *error_log;
> >       int loglevel;
> >
> >       /* Module-specific configuration for server, and defaults... */
> > --- 842,848 ----
> >       /* Log files --- note that transfer log is now in the modules... */
> >
> >       char *error_fname;
> > !     int error_log;
> >       int loglevel;
> >
> >       /* Module-specific configuration for server, and defaults... */
> > diff -rc apache_1.3.3/src/main/http_config.c
> ../apache_1.3.3/src/main/http_config.c
> > *** apache_1.3.3/src/main/http_config.c Fri Sep 25 19:07:08 1998
> > --- ../apache_1.3.3/src/main/http_config.c      Fri Jul 30 11:51:36 1999
> > ***************
> > *** 1408,1414 ****
> >       s->server_admin = DEFAULT_ADMIN;
> >       s->server_hostname = NULL;
> >       s->error_fname = DEFAULT_ERRORLOG;
> > !     s->error_log = stderr;
> >       s->loglevel = DEFAULT_LOGLEVEL;
> >       s->srm_confname = RESOURCE_CONFIG_FILE;
> >       s->access_confname = ACCESS_CONFIG_FILE;
> > --- 1408,1414 ----
> >       s->server_admin = DEFAULT_ADMIN;
> >       s->server_hostname = NULL;
> >       s->error_fname = DEFAULT_ERRORLOG;
> > !     s->error_log = STDERR_FILENO;
> >       s->loglevel = DEFAULT_LOGLEVEL;
> >       s->srm_confname = RESOURCE_CONFIG_FILE;
> >       s->access_confname = ACCESS_CONFIG_FILE;
> > diff -rc apache_1.3.3/src/main/http_log.c
> ../apache_1.3.3/src/main/http_log.c
> > *** apache_1.3.3/src/main/http_log.c    Sat Oct  3 09:28:55 1998
> > --- ../apache_1.3.3/src/main/http_log.c Fri Jul 30 12:02:05 1999
> > ***************
> > *** 195,201 ****
> >             exit(1);
> >         }
> >
> > !       s->error_log = dummy;
> >       }
> >
> >   #ifdef HAVE_SYSLOG
> > --- 195,201 ----
> >             exit(1);
> >         }
> >
> > !       s->error_log = fileno(dummy);
> >       }
> >
> >   #ifdef HAVE_SYSLOG
> > ***************
> > *** 207,213 ****
> >             for (fac = facilities; fac->t_name; fac++) {
> >                 if (!strcasecmp(fname, fac->t_name)) {
> >                     openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID,
> fac->t_val);
> > !                   s->error_log = NULL;
> >                     return;
> >                 }
> >             }
> > --- 207,213 ----
> >             for (fac = facilities; fac->t_name; fac++) {
> >                 if (!strcasecmp(fname, fac->t_name)) {
> >                     openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID,
> fac->t_val);
> > !                   s->error_log = -1;
> >                     return;
> >                 }
> >             }
> > ***************
> > *** 215,226 ****
> >         else
> >             openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, LOG_LOCAL7);
> >
> > !       s->error_log = NULL;
> >       }
> >   #endif
> >       else {
> >         fname = ap_server_root_relative(p, s->error_fname);
> > !         if (!(s->error_log = ap_pfopen(p, fname, "a"))) {
> >               perror("fopen");
> >               fprintf(stderr,"httpd: could not open error log file %s.\n",
> fname);
> >               exit(1);
> > --- 215,233 ----
> >         else
> >             openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, LOG_LOCAL7);
> >
> > !       s->error_log = -1;
> >       }
> >   #endif
> >       else {
> > +         int modeFlags = 0;
> >         fname = ap_server_root_relative(p, s->error_fname);
> > ! #ifdef WIN32
> > !         modeFlags = _S_IREAD | _S_IWRITE;
> > ! #else
> > !         modeFlags = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> S_IWOTH;
> > ! #endif
> > !         if ((s->error_log = ap_popenf(p, fname, O_WRONLY | O_APPEND |
> O_CREAT,
> > !                                         modeFlags)) < 0) {
> >               perror("fopen");
> >               fprintf(stderr,"httpd: could not open error log file %s.\n",
> fname);
> >               exit(1);
> > ***************
> > *** 236,245 ****
> >       open_error_log(s_main, p);
> >
> >       replace_stderr = 1;
> > !     if (s_main->error_log) {
> >         /* replace stderr with this new log */
> >         fflush(stderr);
> > !       if (dup2(fileno(s_main->error_log), STDERR_FILENO) == -1) {
> >             ap_log_error(APLOG_MARK, APLOG_CRIT, s_main,
> >                 "unable to replace stderr with error_log");
> >         } else {
> > --- 243,252 ----
> >       open_error_log(s_main, p);
> >
> >       replace_stderr = 1;
> > !     if (s_main->error_log >= 0) {
> >         /* replace stderr with this new log */
> >         fflush(stderr);
> > !       if (dup2(s_main->error_log, STDERR_FILENO) == -1) {
> >             ap_log_error(APLOG_MARK, APLOG_CRIT, s_main,
> >                 "unable to replace stderr with error_log");
> >         } else {
> > ***************
> > *** 273,281 ****
> >   }
> >
> >   API_EXPORT(void) ap_error_log2stderr(server_rec *s) {
> > !     if (   s->error_log != NULL
> > !         && fileno(s->error_log) != STDERR_FILENO)
> > !         dup2(fileno(s->error_log), STDERR_FILENO);
> >   }
> >
> >   static void log_error_core(const char *file, int line, int level,
> > --- 280,288 ----
> >   }
> >
> >   API_EXPORT(void) ap_error_log2stderr(server_rec *s) {
> > !     if (   s->error_log >= 0
> > !         && s->error_log != STDERR_FILENO)
> > !         dup2(s->error_log, STDERR_FILENO);
> >   }
> >
> >   static void log_error_core(const char *file, int line, int level,
> > ***************
> > *** 285,291 ****
> >       char errstr[MAX_STRING_LEN];
> >       size_t len;
> >       int save_errno = errno;
> > !     FILE *logf;
> >
> >       if (s == NULL) {
> >         /*
> > --- 292,298 ----
> >       char errstr[MAX_STRING_LEN];
> >       size_t len;
> >       int save_errno = errno;
> > !     int logfd;
> >
> >       if (s == NULL) {
> >         /*
> > ***************
> > *** 296,304 ****
> >         if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
> >             ((level & APLOG_LEVELMASK) > DEFAULT_LOGLEVEL))
> >             return;
> > !       logf = stderr;
> >       }
> > !     else if (s->error_log) {
> >         /*
> >          * If we are doing normal logging, don't log messages that are
> >          * above the server log level unless it is a startup/shutdown
> notice
> > --- 303,311 ----
> >         if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
> >             ((level & APLOG_LEVELMASK) > DEFAULT_LOGLEVEL))
> >             return;
> > !       logfd = STDERR_FILENO;
> >       }
> > !     else if (s->error_log >= 0) {
> >         /*
> >          * If we are doing normal logging, don't log messages that are
> >          * above the server log level unless it is a startup/shutdown
> notice
> > ***************
> > *** 306,312 ****
> >         if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
> >             ((level & APLOG_LEVELMASK) > s->loglevel))
> >             return;
> > !       logf = s->error_log;
> >       }
> >       else {
> >         /*
> > --- 313,319 ----
> >         if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
> >             ((level & APLOG_LEVELMASK) > s->loglevel))
> >             return;
> > !       logfd = s->error_log;
> >       }
> >       else {
> >         /*
> > ***************
> > *** 315,324 ****
> >          */
> >         if ((level & APLOG_LEVELMASK) > s->loglevel)
> >             return;
> > !       logf = NULL;
> >       }
> >
> > !     if (logf) {
> >         len = ap_snprintf(errstr, sizeof(errstr), "[%s] ", ap_get_time());
> >       } else {
> >         len = 0;
> > --- 322,331 ----
> >          */
> >         if ((level & APLOG_LEVELMASK) > s->loglevel)
> >             return;
> > !       logfd = -1;
> >       }
> >
> > !     if (logfd > 0) {
> >         len = ap_snprintf(errstr, sizeof(errstr), "[%s] ", ap_get_time());
> >       } else {
> >         len = 0;
> > ***************
> > *** 413,423 ****
> >
> >       len += ap_vsnprintf(errstr + len, sizeof(errstr) - len, fmt, args);
> >
> > !     /* NULL if we are logging to syslog */
> > !     if (logf) {
> > !       fputs(errstr, logf);
> > !       fputc('\n', logf);
> > !       fflush(logf);
> >       }
> >   #ifdef HAVE_SYSLOG
> >       else {
> > --- 420,429 ----
> >
> >       len += ap_vsnprintf(errstr + len, sizeof(errstr) - len, fmt, args);
> >
> > !     /* -1 if we are logging to syslog */
> > !     if (logfd > 0) {
> > !       write(logfd, errstr, len);
> > !         write(logfd, "\n", 1);
> >       }
> >   #ifdef HAVE_SYSLOG
> >       else {
> > diff -rc apache_1.3.3/src/main/util_script.c
> ../apache_1.3.3/src/main/util_script.c
> > *** apache_1.3.3/src/main/util_script.c Sat Oct  3 11:49:32 1998
> > --- ../apache_1.3.3/src/main/util_script.c      Fri Jul 30 11:52:03 1999
> > ***************
> > *** 699,705 ****
> >        * since that is better than allowing errors to go unnoticed.  Don't
> do
> >        * this on Win32, though, since we haven't fork()'d.
> >        */
> > !     r->server->error_log = stderr;
> >   #endif
> >
> >   #ifdef RLIMIT_CPU
> > --- 699,705 ----
> >        * since that is better than allowing errors to go unnoticed.  Don't
> do
> >        * this on Win32, though, since we haven't fork()'d.
> >        */
> > !     r->server->error_log = fileno(stderr);
> >   #endif
> >
> >   #ifdef RLIMIT_CPU

-- 
Michael Douglass
Texas Networking, Inc.

  Any sufficiently advanced bug is indistinguishable for a feature.
    -- from some indian guy

Re: [patch included] Re: use of stdio for error_log

Posted by Scott Hess <sc...@avantgo.com>.
Instead of changing "FILE *error_log" to "int error_log", it would probably
be safer to change the name of the element, say to "int err_log" or "int
error_fd", or something of the sort.  That way any existing code relying on
error_log will break in informative ways.

Later,
scott

----- Original Message -----
From: Michael Douglass <mi...@staff.texas.net>
To: <ne...@apache.org>
Sent: Friday, July 30, 1999 10:50 AM
Subject: [patch included] Re: use of stdio for error_log


>
> Here's the modifications I made to apache 1.3.3 (sorry I didn't grab the
> latest--if there are any changes between 1.3.3 and 1.3.6 I'm certain
> you guys can figure them out).  I also don't know if you really want to
> merge this into the core as there are probably modules out there that
> try to directly access the s->error_log variable (ie. mod_perl--though
> mod_perl's use is very limited and not hard to fix at all).
>
> Here goes; created using gnu diff:
>
> diff -rc apache_1.3.3/src/include/httpd.h
../apache_1.3.3/src/include/httpd.h
> *** apache_1.3.3/src/include/httpd.h    Wed Oct  7 04:19:06 1998
> --- ../apache_1.3.3/src/include/httpd.h Fri Jul 30 11:04:14 1999
> ***************
> *** 842,848 ****
>       /* Log files --- note that transfer log is now in the modules... */
>
>       char *error_fname;
> !     FILE *error_log;
>       int loglevel;
>
>       /* Module-specific configuration for server, and defaults... */
> --- 842,848 ----
>       /* Log files --- note that transfer log is now in the modules... */
>
>       char *error_fname;
> !     int error_log;
>       int loglevel;
>
>       /* Module-specific configuration for server, and defaults... */
> diff -rc apache_1.3.3/src/main/http_config.c
../apache_1.3.3/src/main/http_config.c
> *** apache_1.3.3/src/main/http_config.c Fri Sep 25 19:07:08 1998
> --- ../apache_1.3.3/src/main/http_config.c      Fri Jul 30 11:51:36 1999
> ***************
> *** 1408,1414 ****
>       s->server_admin = DEFAULT_ADMIN;
>       s->server_hostname = NULL;
>       s->error_fname = DEFAULT_ERRORLOG;
> !     s->error_log = stderr;
>       s->loglevel = DEFAULT_LOGLEVEL;
>       s->srm_confname = RESOURCE_CONFIG_FILE;
>       s->access_confname = ACCESS_CONFIG_FILE;
> --- 1408,1414 ----
>       s->server_admin = DEFAULT_ADMIN;
>       s->server_hostname = NULL;
>       s->error_fname = DEFAULT_ERRORLOG;
> !     s->error_log = STDERR_FILENO;
>       s->loglevel = DEFAULT_LOGLEVEL;
>       s->srm_confname = RESOURCE_CONFIG_FILE;
>       s->access_confname = ACCESS_CONFIG_FILE;
> diff -rc apache_1.3.3/src/main/http_log.c
../apache_1.3.3/src/main/http_log.c
> *** apache_1.3.3/src/main/http_log.c    Sat Oct  3 09:28:55 1998
> --- ../apache_1.3.3/src/main/http_log.c Fri Jul 30 12:02:05 1999
> ***************
> *** 195,201 ****
>             exit(1);
>         }
>
> !       s->error_log = dummy;
>       }
>
>   #ifdef HAVE_SYSLOG
> --- 195,201 ----
>             exit(1);
>         }
>
> !       s->error_log = fileno(dummy);
>       }
>
>   #ifdef HAVE_SYSLOG
> ***************
> *** 207,213 ****
>             for (fac = facilities; fac->t_name; fac++) {
>                 if (!strcasecmp(fname, fac->t_name)) {
>                     openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID,
fac->t_val);
> !                   s->error_log = NULL;
>                     return;
>                 }
>             }
> --- 207,213 ----
>             for (fac = facilities; fac->t_name; fac++) {
>                 if (!strcasecmp(fname, fac->t_name)) {
>                     openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID,
fac->t_val);
> !                   s->error_log = -1;
>                     return;
>                 }
>             }
> ***************
> *** 215,226 ****
>         else
>             openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, LOG_LOCAL7);
>
> !       s->error_log = NULL;
>       }
>   #endif
>       else {
>         fname = ap_server_root_relative(p, s->error_fname);
> !         if (!(s->error_log = ap_pfopen(p, fname, "a"))) {
>               perror("fopen");
>               fprintf(stderr,"httpd: could not open error log file %s.\n",
fname);
>               exit(1);
> --- 215,233 ----
>         else
>             openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, LOG_LOCAL7);
>
> !       s->error_log = -1;
>       }
>   #endif
>       else {
> +         int modeFlags = 0;
>         fname = ap_server_root_relative(p, s->error_fname);
> ! #ifdef WIN32
> !         modeFlags = _S_IREAD | _S_IWRITE;
> ! #else
> !         modeFlags = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
S_IWOTH;
> ! #endif
> !         if ((s->error_log = ap_popenf(p, fname, O_WRONLY | O_APPEND |
O_CREAT,
> !                                         modeFlags)) < 0) {
>               perror("fopen");
>               fprintf(stderr,"httpd: could not open error log file %s.\n",
fname);
>               exit(1);
> ***************
> *** 236,245 ****
>       open_error_log(s_main, p);
>
>       replace_stderr = 1;
> !     if (s_main->error_log) {
>         /* replace stderr with this new log */
>         fflush(stderr);
> !       if (dup2(fileno(s_main->error_log), STDERR_FILENO) == -1) {
>             ap_log_error(APLOG_MARK, APLOG_CRIT, s_main,
>                 "unable to replace stderr with error_log");
>         } else {
> --- 243,252 ----
>       open_error_log(s_main, p);
>
>       replace_stderr = 1;
> !     if (s_main->error_log >= 0) {
>         /* replace stderr with this new log */
>         fflush(stderr);
> !       if (dup2(s_main->error_log, STDERR_FILENO) == -1) {
>             ap_log_error(APLOG_MARK, APLOG_CRIT, s_main,
>                 "unable to replace stderr with error_log");
>         } else {
> ***************
> *** 273,281 ****
>   }
>
>   API_EXPORT(void) ap_error_log2stderr(server_rec *s) {
> !     if (   s->error_log != NULL
> !         && fileno(s->error_log) != STDERR_FILENO)
> !         dup2(fileno(s->error_log), STDERR_FILENO);
>   }
>
>   static void log_error_core(const char *file, int line, int level,
> --- 280,288 ----
>   }
>
>   API_EXPORT(void) ap_error_log2stderr(server_rec *s) {
> !     if (   s->error_log >= 0
> !         && s->error_log != STDERR_FILENO)
> !         dup2(s->error_log, STDERR_FILENO);
>   }
>
>   static void log_error_core(const char *file, int line, int level,
> ***************
> *** 285,291 ****
>       char errstr[MAX_STRING_LEN];
>       size_t len;
>       int save_errno = errno;
> !     FILE *logf;
>
>       if (s == NULL) {
>         /*
> --- 292,298 ----
>       char errstr[MAX_STRING_LEN];
>       size_t len;
>       int save_errno = errno;
> !     int logfd;
>
>       if (s == NULL) {
>         /*
> ***************
> *** 296,304 ****
>         if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
>             ((level & APLOG_LEVELMASK) > DEFAULT_LOGLEVEL))
>             return;
> !       logf = stderr;
>       }
> !     else if (s->error_log) {
>         /*
>          * If we are doing normal logging, don't log messages that are
>          * above the server log level unless it is a startup/shutdown
notice
> --- 303,311 ----
>         if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
>             ((level & APLOG_LEVELMASK) > DEFAULT_LOGLEVEL))
>             return;
> !       logfd = STDERR_FILENO;
>       }
> !     else if (s->error_log >= 0) {
>         /*
>          * If we are doing normal logging, don't log messages that are
>          * above the server log level unless it is a startup/shutdown
notice
> ***************
> *** 306,312 ****
>         if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
>             ((level & APLOG_LEVELMASK) > s->loglevel))
>             return;
> !       logf = s->error_log;
>       }
>       else {
>         /*
> --- 313,319 ----
>         if (((level & APLOG_LEVELMASK) != APLOG_NOTICE) &&
>             ((level & APLOG_LEVELMASK) > s->loglevel))
>             return;
> !       logfd = s->error_log;
>       }
>       else {
>         /*
> ***************
> *** 315,324 ****
>          */
>         if ((level & APLOG_LEVELMASK) > s->loglevel)
>             return;
> !       logf = NULL;
>       }
>
> !     if (logf) {
>         len = ap_snprintf(errstr, sizeof(errstr), "[%s] ", ap_get_time());
>       } else {
>         len = 0;
> --- 322,331 ----
>          */
>         if ((level & APLOG_LEVELMASK) > s->loglevel)
>             return;
> !       logfd = -1;
>       }
>
> !     if (logfd > 0) {
>         len = ap_snprintf(errstr, sizeof(errstr), "[%s] ", ap_get_time());
>       } else {
>         len = 0;
> ***************
> *** 413,423 ****
>
>       len += ap_vsnprintf(errstr + len, sizeof(errstr) - len, fmt, args);
>
> !     /* NULL if we are logging to syslog */
> !     if (logf) {
> !       fputs(errstr, logf);
> !       fputc('\n', logf);
> !       fflush(logf);
>       }
>   #ifdef HAVE_SYSLOG
>       else {
> --- 420,429 ----
>
>       len += ap_vsnprintf(errstr + len, sizeof(errstr) - len, fmt, args);
>
> !     /* -1 if we are logging to syslog */
> !     if (logfd > 0) {
> !       write(logfd, errstr, len);
> !         write(logfd, "\n", 1);
>       }
>   #ifdef HAVE_SYSLOG
>       else {
> diff -rc apache_1.3.3/src/main/util_script.c
../apache_1.3.3/src/main/util_script.c
> *** apache_1.3.3/src/main/util_script.c Sat Oct  3 11:49:32 1998
> --- ../apache_1.3.3/src/main/util_script.c      Fri Jul 30 11:52:03 1999
> ***************
> *** 699,705 ****
>        * since that is better than allowing errors to go unnoticed.  Don't
do
>        * this on Win32, though, since we haven't fork()'d.
>        */
> !     r->server->error_log = stderr;
>   #endif
>
>   #ifdef RLIMIT_CPU
> --- 699,705 ----
>        * since that is better than allowing errors to go unnoticed.  Don't
do
>        * this on Win32, though, since we haven't fork()'d.
>        */
> !     r->server->error_log = fileno(stderr);
>   #endif
>
>   #ifdef RLIMIT_CPU