httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dean Gaudet <dgau...@arctic.org>
Subject Re: [PATCH] Cleaning up FILE,LINE,errno in aplog_error()
Date Thu, 02 Oct 1997 00:29:38 GMT
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;


Mime
View raw message