You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rodent of Unusual Size <co...@decus.org> on 1997/10/01 21:55:09 UTC
[PATCH] Cleaning up FILE,LINE,errno in aplog_error()
I spent a little time on this to-day to try to address the issues of
filename, line number, and errno information showing up in the error
log.
The solution I present here has the advantage that it won't change
any of the exiting message until the corresponding aplog_error()
calls are altered.
The solution I have here is to add a Boolean argument to
aplog_error() to indicate whether errno information should be
included. Also, if the filename is NULL or the line number is -,
those fields won't be included. Finally, the following macros to
control the behaviour have been added: APLOG_NOMARK,
APLOG_MARK_NOERRNO, and APLOG_NOMARK_NOERRNO.
I've also added a patch for mod_unique_id.c that demonstrates the
use.
Hopefully this will address some of the issues with logging before
1.3b1..
#ken P-)}
Index: main/http_log.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_log.h,v
retrieving revision 1.13
diff -u -w -r1.13 http_log.h
--- http_log.h 1997/09/12 20:13:08 1.13
+++ http_log.h 1997/10/01 20:10:49
@@ -59,7 +59,10 @@
#define APLOG_INFO 6 /* informational */
#define APLOG_DEBUG 7 /* debug-level messages */
#define DEFAULT_LOGLEVEL APLOG_ERR
-#define APLOG_MARK __FILE__,__LINE__
+#define APLOG_MARK __FILE__,__LINE__,1
+#define APLOG_MARK_NOERRNO __FILE__,__LINE__,0
+#define APLOG_NOMARK NULL,0,1
+#define APLOG_NOMARK_NOERRNO NULL,0,0
typedef struct _trans {
char *t_name;
@@ -67,8 +70,9 @@
} TRANS;
void open_logs (server_rec *, pool *p);
-API_EXPORT(void) aplog_error(const char *file, int line, int level,
- const server_rec *s, const char *fmt, ...);
+API_EXPORT(void) aplog_error(const char *file, int line, int incl_errno,
+ int level, const server_rec *s,
+ const char *fmt, ...);
API_EXPORT(void) error_log2stderr (server_rec *);
void log_pid (pool *p, char *fname);
Index: main/http_log.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_log.c,v
retrieving revision 1.34
diff -u -w -r1.34 http_log.c
--- http_log.c 1997/09/12 20:37:33 1.34
+++ http_log.c 1997/10/01 20:10:54
@@ -247,27 +247,49 @@
dup2(fileno(s->error_log),STDERR_FILENO);
}
-API_EXPORT(void) aplog_error (const char *file, int line, int level,
- const server_rec *s, const char *fmt, ...)
+API_EXPORT(void) aplog_error (const char *file, int line, int incl_errno,
+ int level, const server_rec *s,
+ const char *fmt, ...)
{
va_list args;
char errstr[MAX_STRING_LEN];
+ char fstr[48];
+ char serrno[48], sline[48];
+ char *p_file, *p_line, *p_errno;
+ const char *fname;
static TRANS *pname = priorities;
- if (level > s->loglevel)
+ if (level > s->loglevel) {
return;
+ }
+
+ ap_snprintf(serrno, sizeof(serrno), "%d", errno);
+ ap_snprintf(sline, sizeof(sline), "%d", line);
+ p_errno = (incl_errno) ? "%s: %s: " : "%.0s%.0s";
+ p_line = (line != 0) ? "%s: " : "%.0s";
+ if (file == NULL) {
+ p_file = "%.0s";
+ fname = "";
+ }
+ else {
+ p_file = "%s: ";
+ fname = file;
+ }
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);
+ ap_snprintf(fstr, sizeof(fstr), "[%%s] %s%s%s", p_errno, p_file,
+ p_line);
+ ap_snprintf(errstr, sizeof(errstr), fstr,
+ pname[level].t_name, serrno, strerror(errno), fname, sline);
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));
+ ap_snprintf(fstr, sizeof(fstr), "[%%s] %s", p_errno);
+ ap_snprintf(errstr, sizeof(errstr), fstr,
+ pname[level].t_name, serrno, strerror(errno));
break;
case APLOG_INFO:
case APLOG_ERR:
@@ -319,7 +341,7 @@
API_EXPORT(void) log_unixerr (const char *routine, const char *file,
const char *msg, server_rec *s)
{
- aplog_error(file, 0, APLOG_ERR, s, msg);
+ aplog_error(file, 0, 1, APLOG_ERR, s, msg);
}
API_EXPORT(void) log_printf (const server_rec *s, const char *fmt, ...)
Index: modules/standard/mod_unique_id.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/standard/mod_unique_id.c,v
retrieving revision 1.4
diff -u -w -r1.4 mod_unique_id.c
--- mod_unique_id.c 1997/09/16 04:49:50 1.4
+++ mod_unique_id.c 1997/10/01 20:10:58
@@ -145,7 +145,7 @@
*/
if (XtOffsetOf(unique_id_rec, counter) + sizeof(cur_unique_id.counter)
!= 14) {
- aplog_error(APLOG_MARK, APLOG_ALERT, s,
+ aplog_error(APLOG_MARK_NOERRNO, APLOG_ALERT, s,
"mod_unique_id: sorry the size assumptions are wrong "
"in mod_unique_id.c, please remove it from your server "
"or fix the code!");
@@ -158,7 +158,7 @@
* be unique as the physical address of the machine
*/
if (gethostname(str, sizeof(str) - 1) != 0) {
- aplog_error(APLOG_MARK, APLOG_ALERT, s,
+ aplog_error(APLOG_MARK_NOERRNO, APLOG_ALERT, s,
"gethostname: mod_unique_id requires the hostname of the server");
exit(1);
}
@@ -171,7 +171,7 @@
global_in_addr = ((struct in_addr *) hent->h_addr_list[0])->s_addr;
- aplog_error(APLOG_MARK, APLOG_INFO, s,
+ aplog_error(APLOG_NOMARK_NOERRNO, APLOG_INFO, s,
"mod_unique_id: using ip addr %s",
inet_ntoa(*(struct in_addr *) hent->h_addr_list[0]));
Re: [PATCH] Cleaning up FILE,LINE,errno in aplog_error()
Posted by Ben Laurie <be...@algroup.co.uk>.
Dean Gaudet wrote:
>
> Can I get some votes on this please? I want to clean this up today. If
> people support this then I'll go stick in the APLOG_NOERRNO all over the
> place
No time to test, but a conceptual +1.
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 994 6435|Apache Group member
Freelance Consultant |Fax: +44 (181) 994 6472|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |Apache-SSL author
A.L. Digital Ltd, |http://www.algroup.co.uk/Apache-SSL
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
Re: [PATCH] Cleaning up FILE,LINE,errno in aplog_error()
Posted by Dean Gaudet <dg...@arctic.org>.
Can I get some votes on this please? I want to clean this up today. If
people support this then I'll go stick in the APLOG_NOERRNO all over the
place
Dean
On Wed, 1 Oct 1997, Dean Gaudet wrote:
> On Wed, 1 Oct 1997, Rodent of Unusual Size wrote:
>
> > I spent a little time on this to-day to try to address the issues of
> > filename, line number, and errno information showing up in the error
> > log.
>
> Good start ... but here's my approach.
>
> Stick an APLOG_NOERRNO bit into the level paramter, this saves one
> parameter on each call (and the level and errno are constants so it
> actually comes for free).
>
> When server_conf is NULL use stderr. Necessary to do some things during
> startup and other odd cases (i.e. SEGV).
>
> Do the null filename thing like Ken did.
>
> Do it all with a lot less string copying -- use snprintf incrementally to
> build up the entire buffer.
>
> Fix a buffer overflow with syslog stuff using sprintf().
>
> I tweaked a bunch of stuff in http_main.c as well to show how you'd use
> this.
>
> Dean
>
> Index: main/http_log.c
> ===================================================================
> RCS file: /export/home/cvs/apachen/src/main/http_log.c,v
> retrieving revision 1.34
> diff -u -r1.34 http_log.c
> --- http_log.c 1997/09/12 20:37:33 1.34
> +++ http_log.c 1997/10/02 00:23:05
> @@ -253,48 +253,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)
> {
> Index: main/http_log.h
> ===================================================================
> RCS file: /export/home/cvs/apachen/src/main/http_log.h,v
> retrieving revision 1.13
> diff -u -r1.13 http_log.h
> --- http_log.h 1997/09/12 20:13:08 1.13
> +++ http_log.h 1997/10/02 00:23:05
> @@ -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 {
> Index: main/http_main.c
> ===================================================================
> RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
> retrieving revision 1.226
> diff -u -r1.226 http_main.c
> --- http_main.c 1997/09/25 01:03:22 1.226
> +++ http_main.c 1997/10/02 00:23:08
> @@ -715,7 +715,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
> @@ -1331,7 +1332,6 @@
>
> static void setup_shared_mem(void)
> {
> - char errstr[MAX_STRING_LEN];
> struct shmid_ds shmbuf;
> #ifdef MOVEBREAK
> char *obrk;
> @@ -1340,18 +1340,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
> /*
> @@ -1364,30 +1364,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);
> }
> }
> }
> @@ -1396,12 +1395,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);
> @@ -1410,8 +1406,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;
> @@ -1752,21 +1748,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);
> @@ -1777,7 +1773,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;
> @@ -1914,7 +1910,8 @@
> ap_snprintf(emsg, sizeof(emsg),
> "httpd: caught SIGBUS, attempting to dump core in %s",
> coredump_dir);
> - aplog_error(APLOG_MARK, APLOG_INFO, server_conf, emsg);
> + /* don't use any specific server record, no idea if it's been messed up */
> + aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, NULL, emsg);
> chdir(coredump_dir);
> abort();
> exit(1);
> @@ -1927,7 +1924,8 @@
> ap_snprintf(emsg, sizeof(emsg),
> "httpd: caught SIGSEGV, attempting to dump core in %s",
> coredump_dir);
> - aplog_error(APLOG_MARK, APLOG_INFO, server_conf, emsg);
> + /* don't use any specific server record, no idea if it's been messed up */
> + aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_INFO, NULL, emsg);
> chdir(coredump_dir);
> abort();
> exit(1);
> @@ -2266,7 +2264,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
>
> @@ -2871,7 +2869,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);
> @@ -3250,7 +3248,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;
> @@ -3259,7 +3257,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);
> @@ -3364,11 +3362,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;
>
> @@ -3408,7 +3406,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,
> @@ -3443,7 +3441,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 */
> @@ -3473,7 +3471,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 */
> @@ -3500,7 +3498,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;
>
>
Re: [PATCH] Cleaning up FILE,LINE,errno in aplog_error()
Posted by Dean Gaudet <dg...@arctic.org>.
On Wed, 1 Oct 1997, Rodent of Unusual Size wrote:
> I spent a little time on this to-day to try to address the issues of
> filename, line number, and errno information showing up in the error
> log.
Good start ... but here's my approach.
Stick an APLOG_NOERRNO bit into the level paramter, this saves one
parameter on each call (and the level and errno are constants so it
actually comes for free).
When server_conf is NULL use stderr. Necessary to do some things during
startup and other odd cases (i.e. SEGV).
Do the null filename thing like Ken did.
Do it all with a lot less string copying -- use snprintf incrementally to
build up the entire buffer.
Fix a buffer overflow with syslog stuff using sprintf().
I tweaked a bunch of stuff in http_main.c as well to show how you'd use
this.
Dean
Index: main/http_log.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_log.c,v
retrieving revision 1.34
diff -u -r1.34 http_log.c
--- http_log.c 1997/09/12 20:37:33 1.34
+++ http_log.c 1997/10/02 00:23:05
@@ -253,48 +253,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)
{
Index: main/http_log.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_log.h,v
retrieving revision 1.13
diff -u -r1.13 http_log.h
--- http_log.h 1997/09/12 20:13:08 1.13
+++ http_log.h 1997/10/02 00:23:05
@@ -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 {
Index: main/http_main.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
retrieving revision 1.226
diff -u -r1.226 http_main.c
--- http_main.c 1997/09/25 01:03:22 1.226
+++ http_main.c 1997/10/02 00:23:08
@@ -715,7 +715,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
@@ -1331,7 +1332,6 @@
static void setup_shared_mem(void)
{
- char errstr[MAX_STRING_LEN];
struct shmid_ds shmbuf;
#ifdef MOVEBREAK
char *obrk;
@@ -1340,18 +1340,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
/*
@@ -1364,30 +1364,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);
}
}
}
@@ -1396,12 +1395,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);
@@ -1410,8 +1406,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;
@@ -1752,21 +1748,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);
@@ -1777,7 +1773,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;
@@ -1914,7 +1910,8 @@
ap_snprintf(emsg, sizeof(emsg),
"httpd: caught SIGBUS, attempting to dump core in %s",
coredump_dir);
- aplog_error(APLOG_MARK, APLOG_INFO, server_conf, emsg);
+ /* don't use any specific server record, no idea if it's been messed up */
+ aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, NULL, emsg);
chdir(coredump_dir);
abort();
exit(1);
@@ -1927,7 +1924,8 @@
ap_snprintf(emsg, sizeof(emsg),
"httpd: caught SIGSEGV, attempting to dump core in %s",
coredump_dir);
- aplog_error(APLOG_MARK, APLOG_INFO, server_conf, emsg);
+ /* don't use any specific server record, no idea if it's been messed up */
+ aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_INFO, NULL, emsg);
chdir(coredump_dir);
abort();
exit(1);
@@ -2266,7 +2264,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
@@ -2871,7 +2869,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);
@@ -3250,7 +3248,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;
@@ -3259,7 +3257,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);
@@ -3364,11 +3362,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;
@@ -3408,7 +3406,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,
@@ -3443,7 +3441,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 */
@@ -3473,7 +3471,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 */
@@ -3500,7 +3498,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;