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 Sun, 05 Oct 1997 00:51:48 GMT
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;
> 
> 


Mime
View raw message