fielding    97/10/05 00:47:48

  Modified:    src      CHANGES
               src/main http_log.c http_log.h http_main.c
  Log:
  Reduce what is written by aplog_error() by excluding filename if it
  is NULL, line number if it is "-", and errno if the new APLOG_NOERRNO bit
  is set in level parameter.  Do it all with a lot less string copying --
  use snprintf incrementally to build up the entire buffer.
  When server_conf is NULL use stderr, since that is necessary to do some
  things during startup and other odd cases (i.e. SEGV).
  Fixed a buffer overflow with syslog stuff using sprintf().
  
  Submitted by: Ken Coar and Dean Gaudet
  Reviewed by:  Roy Fielding
  
  Revision  Changes    Path
  1.457     +3 -0      apachen/src/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /export/home/cvs/apachen/src/CHANGES,v
  retrieving revision 1.456
  retrieving revision 1.457
  diff -u -r1.456 -r1.457
  --- CHANGES   1997/10/05 02:22:22     1.456
  +++ CHANGES   1997/10/05 07:47:41     1.457
  @@ -168,6 +168,9 @@
        on a per-server basis using the LogLevel directive. Conversion
        of log_*() in progress. [Randy Terbush]
   
  +  *) Further enhance aplog_error() to not log filename, line number, and
  +     errno information when it isn't applicable. [Ken Coar, Dean Gaudet]
  +
     *) Canonicalise filenames under Win32. Short filenames are
        converted to long ones. Backslashes are converted to forward
        slashes. Case is converted to lower. Parts of URLs that do not
  
  
  
  1.36      +37 -29    apachen/src/main/http_log.c
  
  Index: http_log.c
  ===================================================================
  RCS file: /export/home/cvs/apachen/src/main/http_log.c,v
  retrieving revision 1.35
  retrieving revision 1.36
  diff -u -r1.35 -r1.36
  --- http_log.c        1997/10/02 05:10:34     1.35
  +++ http_log.c        1997/10/05 07:47:45     1.36
  @@ -280,48 +280,56 @@
       va_list args;
       char errstr[MAX_STRING_LEN];
       static TRANS *pname = priorities;
  -    
  +    size_t len;
  +    int save_errno = errno;
  +    FILE *logf;
   
  -    if (level > s->loglevel)
  +    if (s && (level & APLOG_LEVELMASK) > s->loglevel)
        return;
   
  -    switch (s->loglevel) {
  -    case APLOG_DEBUG:
  -     ap_snprintf(errstr, sizeof(errstr), "[%s] %d: %s: %s: %d: ",
  -                 pname[level].t_name, errno, strerror(errno), file, line);
  -     break;
  -    case APLOG_EMERG:
  -    case APLOG_CRIT:
  -    case APLOG_ALERT:
  -     ap_snprintf(errstr, sizeof(errstr), "[%s] %d: %s: ",
  -                 pname[level].t_name, errno, strerror(errno));
  -     break;
  -    case APLOG_INFO:
  -    case APLOG_ERR:
  -    case APLOG_WARNING:
  -    case APLOG_NOTICE:
  -     ap_snprintf(errstr, sizeof(errstr), "[%s] ", pname[level].t_name);
  -     break;
  +    if (!s) {
  +     logf = stderr;
  +    } else if (s && s->error_log) {
  +     logf = s->error_log;
  +    } else {
  +     logf = NULL;
  +    }
  +
  +    if (logf) {
  +     len = ap_snprintf(errstr, sizeof(errstr), "[%s] ", get_time());
  +    } else {
  +     len = 0;
  +    }
  +
  +    len += ap_snprintf(errstr + len, sizeof(errstr) - len,
  +         "[%s] ", pname[level & APLOG_LEVELMASK].t_name);
  +
  +    if (!(level & APLOG_NOERRNO)) {
  +     len += ap_snprintf(errstr + len, sizeof(errstr) - len,
  +             "%d: %s: ", save_errno, strerror(save_errno));
       }
  -     
  +    if (file && (level & APLOG_LEVELMASK) == APLOG_DEBUG) {
  +     len += ap_snprintf(errstr + len, sizeof(errstr) - len,
  +             "%s: %d: ", file, line);
  +    }
  +
       va_start(args, fmt);
  +    len += ap_vsnprintf(errstr + len, sizeof(errstr) - len, fmt, args);
  +    va_end(args);
   
       /* NULL if we are logging to syslog */
  -    if (s->error_log) {
  -     fprintf(s->error_log, "[%s] %s", get_time(), errstr);
  -     vfprintf(s->error_log, fmt, args);
  -     fprintf(s->error_log, "\n");
  -     fflush(s->error_log);
  +    if (logf) {
  +     fputs(errstr, logf);
  +     fputc('\n', logf);
  +     fflush(logf);
       }
   #ifdef HAVE_SYSLOG
       else {
  -     vsprintf(errstr + strlen(errstr), fmt, args);
  -     syslog(level, "%s", errstr);
  +     syslog(level & APLOG_LEVELMASK, "%s", errstr);
       }
   #endif
  -    
  -    va_end(args);
   }
  +    
   
   void log_pid (pool *p, char *pid_fname)
   {
  
  
  
  1.14      +2 -0      apachen/src/main/http_log.h
  
  Index: http_log.h
  ===================================================================
  RCS file: /export/home/cvs/apachen/src/main/http_log.h,v
  retrieving revision 1.13
  retrieving revision 1.14
  diff -u -r1.13 -r1.14
  --- http_log.h        1997/09/12 20:13:08     1.13
  +++ http_log.h        1997/10/05 07:47:45     1.14
  @@ -59,6 +59,8 @@
   #define      APLOG_INFO      6       /* informational */
   #define      APLOG_DEBUG     7       /* debug-level messages */
   #define DEFAULT_LOGLEVEL     APLOG_ERR
  +#define APLOG_LEVELMASK 15   /* mask off the level value */
  +#define APLOG_NOERRNO        16
   #define APLOG_MARK   __FILE__,__LINE__
   
   typedef struct _trans {
  
  
  
  1.231     +34 -38    apachen/src/main/http_main.c
  
  Index: http_main.c
  ===================================================================
  RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
  retrieving revision 1.230
  retrieving revision 1.231
  diff -u -r1.230 -r1.231
  --- http_main.c       1997/10/05 02:06:37     1.230
  +++ http_main.c       1997/10/05 07:47:46     1.231
  @@ -719,7 +719,8 @@
       }
   
       if (!current_conn->keptalive)
  -     aplog_error(APLOG_MARK, APLOG_DEBUG, current_conn->server, errstr);
  +     aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_DEBUG,
  +                 current_conn->server, errstr);
   
       if (timeout_req) {
        /* Someone has asked for this transaction to just be aborted
  @@ -1336,7 +1337,6 @@
   
   static void setup_shared_mem(void)
   {
  -    char errstr[MAX_STRING_LEN];
       struct shmid_ds shmbuf;
   #ifdef MOVEBREAK
       char *obrk;
  @@ -1345,18 +1345,18 @@
       if ((shmid = shmget(shmkey, SCOREBOARD_SIZE, IPC_CREAT | SHM_R | SHM_W)) 
== -1) {
   #ifdef LINUX
        if (errno == ENOSYS) {
  -         fprintf(stderr,
  +         aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_EMERG, server_conf,
                    "httpd: Your kernel was built without CONFIG_SYSVIPC\n"
                    "httpd: please consult the Apache FAQ for details\n");
        }
   #endif
  -     perror("shmget");
  -     fprintf(stderr, "httpd: Could not call shmget\n");
  +     aplog_error(APLOG_MARK, APLOG_EMERG, server_conf,
  +                 "could not call shmget");
        exit(1);
       }
   
  -    ap_snprintf(errstr, sizeof(errstr), "created shared memory segment #%d", 
shmid);
  -    aplog_error(APLOG_MARK, APLOG_INFO, server_conf, errstr);
  +    aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_INFO, server_conf,
  +             "created shared memory segment #%d", shmid);
   
   #ifdef MOVEBREAK
       /*
  @@ -1369,30 +1369,29 @@
        * attach the segment and then move break back down. Ugly
        */
       if ((obrk = sbrk(MOVEBREAK)) == (char *) -1) {
  -     perror("sbrk");
  -     fprintf(stderr, "httpd: Could not move break\n");
  +     aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +         "sbrk() could not move break");
       }
   #endif
   
   #define BADSHMAT     ((scoreboard *)(-1))
       if ((scoreboard_image = (scoreboard *) shmat(shmid, 0, 0)) == BADSHMAT) {
  -     perror("shmat");
  -     fprintf(stderr, "httpd: Could not call shmat\n");
  +     aplog_error(APLOG_MARK, APLOG_EMERG, server_conf, "shmat error");
        /*
         * We exit below, after we try to remove the segment
         */
       }
       else {                   /* only worry about permissions if we attached 
the segment */
        if (shmctl(shmid, IPC_STAT, &shmbuf) != 0) {
  -         perror("shmctl");
  -         fprintf(stderr, "httpd: Could not stat segment #%d\n", shmid);
  +         aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +             "shmctl() could not stat segment #%d", shmid);
        }
        else {
            shmbuf.shm_perm.uid = user_id;
            shmbuf.shm_perm.gid = group_id;
            if (shmctl(shmid, IPC_SET, &shmbuf) != 0) {
  -             perror("shmctl");
  -             fprintf(stderr, "httpd: Could not set segment #%d\n", shmid);
  +             aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +                 "shmctl() could not set segment #%d", shmid);
            }
        }
       }
  @@ -1401,12 +1400,9 @@
        * (small) tables.
        */
       if (shmctl(shmid, IPC_RMID, NULL) != 0) {
  -     perror("shmctl");
  -     fprintf(stderr, "httpd: Could not delete segment #%d\n", shmid);
  -     ap_snprintf(errstr, sizeof(errstr),
  -                 "could not remove shared memory segment #%d", shmid);
        aplog_error(APLOG_MARK, APLOG_WARNING, server_conf,
  -                 "shmctl: IPC_RMID: %s", errstr);
  +             "shmctl: IPC_RMID: could not remove shared memory segment #%d",
  +             shmid);
       }
       if (scoreboard_image == BADSHMAT)        /* now bailout */
        exit(1);
  @@ -1415,8 +1411,8 @@
       if (obrk == (char *) -1)
        return;                 /* nothing else to do */
       if (sbrk(-(MOVEBREAK)) == (char *) -1) {
  -     perror("sbrk");
  -     fprintf(stderr, "httpd: Could not move break back\n");
  +     aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +         "sbrk() could not move break back");
       }
   #endif
       scoreboard_image->global.exit_generation = 0;
  @@ -1757,21 +1753,21 @@
            switch (tries) {
            case 1:
                /* perhaps it missed the SIGHUP, lets try again */
  -             aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +             aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
                    "child process %d did not exit, sending another SIGHUP",
                            pid);
                kill(pid, SIGHUP);
                break;
            case 2:
                /* ok, now it's being annoying */
  -             aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +             aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
                   "child process %d still did not exit, sending a SIGTERM",
                            pid);
                kill(pid, SIGTERM);
                break;
            case 3:
                /* die child scum */
  -             aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +             aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
                   "child process %d still did not exit, sending a SIGKILL",
                            pid);
                kill(pid, SIGKILL);
  @@ -1782,7 +1778,7 @@
                 * exited, we will likely fail to bind to the port
                 * after the restart.
                 */
  -             aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +             aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
                            "could not make child process %d exit, "
                            "attempting to continue anyway", pid);
                break;
  @@ -1952,7 +1948,7 @@
       ap_snprintf(emsg, sizeof(emsg),
                "httpd: caught %s, attempting to dump core in %s",
                s, coredump_dir);
  -    aplog_error(APLOG_MARK, APLOG_INFO, server_conf, emsg);
  +    aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_INFO, server_conf, emsg);
       chdir(coredump_dir);
       abort();
       exit(1);
  @@ -2325,7 +2321,7 @@
       }
       p += ap_snprintf(p, sizeof(buf) - (p - buf), " %ux%u",
                     total, count[VHASH_TABLE_SIZE - 1]);
  -    aplog_error(APLOG_MARK, APLOG_DEBUG, server_conf, buf);
  +    aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_DEBUG, server_conf, buf);
   }
   #endif
   
  @@ -2930,7 +2926,7 @@
   #ifdef LINUX
                    if (errno == EFAULT) {
                        aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  -                                 "select: (listen) fatal, exiting");
  +                                 "select: (listen) fatal, child exiting");
                        child_exit_modules(pconf, server_conf);
                        destroy_pool(pconf);
                        exit(1);
  @@ -3309,7 +3305,7 @@
            static int reported = 0;
   
            if (!reported) {
  -             aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +             aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
                            "server reached MaxClients setting, consider"
                            " raising the MaxClients setting");
                reported = 1;
  @@ -3318,7 +3314,7 @@
        }
        else {
            if (idle_spawn_rate >= 4) {
  -             aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
  +             aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
                    "server seems busy, spawning %d children (you may need "
                        "to increase StartServers, or Min/MaxSpareServers)",
                            idle_spawn_rate);
  @@ -3423,11 +3419,11 @@
            hold_off_on_exponential_spawning = 10;
        }
   
  -     aplog_error(APLOG_MARK, APLOG_INFO, server_conf,
  +     aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_INFO, server_conf,
                    "Apache HTTP Server version: %s", SERVER_VERSION);
  -     aplog_error(APLOG_MARK, APLOG_INFO, server_conf,
  +     aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_INFO, server_conf,
                    "Server built: %s", SERVER_BUILT);
  -     aplog_error(APLOG_MARK, APLOG_INFO, server_conf,
  +     aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_INFO, server_conf,
                    "Server configured -- resuming normal operations");
        restart_pending = shutdown_pending = 0;
   
  @@ -3467,7 +3463,7 @@
                     * scoreboard.  Somehow we don't know about this
                     * child.
                     */
  -                 aplog_error(APLOG_MARK, APLOG_WARNING, server_conf,
  +                 aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING, 
server_conf,
                                "long lost child came home! (pid %d)", pid);
                }
                /* Don't perform idle maintenance when a child dies,
  @@ -3502,7 +3498,7 @@
                aplog_error(APLOG_MARK, APLOG_WARNING, server_conf, "killpg 
SIGTERM");
            }
            reclaim_child_processes(2);         /* Start with SIGTERM */
  -         aplog_error(APLOG_MARK, APLOG_NOTICE, server_conf,
  +         aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, server_conf,
                        "httpd: caught SIGTERM, shutting down");
   
            /* Clear the pool - including any registered cleanups */
  @@ -3532,7 +3528,7 @@
            scoreboard_image->global.exit_generation = generation;
            update_scoreboard_global();
   
  -         aplog_error(APLOG_MARK, APLOG_NOTICE, server_conf,
  +         aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, server_conf,
                        "SIGUSR1 received.  Doing graceful restart");
   
            /* kill off the idle ones */
  @@ -3559,7 +3555,7 @@
                aplog_error(APLOG_MARK, APLOG_WARNING, server_conf, "killpg 
SIGHUP");
            }
            reclaim_child_processes(1);         /* Not when just starting up */
  -         aplog_error(APLOG_MARK, APLOG_NOTICE, server_conf,
  +         aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, server_conf,
                        "SIGHUP received.  Attempting to restart");
        }
        ++generation;
  
  
  

Reply via email to