made sure no call to exit or die is done once rsyslogd is running

3 files changed, 47 insertions, 41 deletions

diff --git a/NEWS b/NEWS
index 7d15e282..20489427 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,11 @@ Version 1.11.2 (RGer), 2005-10-20
   by an alarm handler, which could lead to all sorts of bad things. This
   has been changed now. See comments in syslogd.c/domark() for details.
 - improved debug output for property-based filters
+- not a code change, but: I have checked all exit()s to make sure that
+  none occurs once rsyslogd has started up. Even in unusual conditions
+  (like low-memory conditions) rsyslogd somehow remains active. Of course,
+  it might loose a message or two, but at least it does not abort and it
+  can also recover when the condition no longer persist.
 ---------------------------------------------------------------------------
 Version 1.11.1 (RGer), 2005-10-19
 - support for BSD-style program name and host blocks
diff --git a/doc/bugs.html b/doc/bugs.html
index f3592b0c..1903ba5b 100644
--- a/doc/bugs.html
+++ b/doc/bugs.html
@@ -41,14 +41,6 @@ names...).</p>
 <p>This format is actually not 100% compatible with stock syslogd - the

    date is missing. Will be fixed soon and can also be fixed just via

    the proper template. Anyone up for this? ;)</p>

-<h2>CALLS to exit()</h2>

-<p>I have removed all calls to exit() I have introduced. However, there are

-   still some left from the original code. Most of them are OK, being done

-   on initial startup when a severe error happens. There are some, I think,

-   that might happen during runtime when a memory shortage is detected. We

-   should look into this issues. Finding some smarter code would allow

-   rsyslog to become even more fault-tolerant (but obviously, there is always

-   a cost, in this case most probably the loss of at least one message).</p>

 <h2>SIGPIPE HANDLING</h2>

 <p>Currently, SIGPIPE is ignored. This is necessary to handle broken TCP

   connections. We should further look into this issue and see which other

diff --git a/syslogd.c b/syslogd.c
index d36549a9..afe78364 100644
--- a/syslogd.c
+++ b/syslogd.c
@@ -2760,7 +2760,7 @@ int main(int argc, char **argv)
 			printf("\tFEATURE_DEBUG (debug build, slow code)\n");
 #endif
 			printf("\nSee http://www.rsyslog.com for more information.\n");
-			exit(0);
+			exit(0); /* exit for -v option - so this is a "good one" */
 		case 'w':		/* disable disallowed host warnigs */
 			option_DisallowWarning = 0;
 			break;
@@ -2791,7 +2791,7 @@ int main(int argc, char **argv)
 				 * initialized or the klogd won't be able to flush its
 				 * logs.  -Joey
 				 */
-				exit(1);
+				exit(1); /* "good" exit - after forking, not diasabling anything */
 			}
 			num_fds = getdtablesize();
 			for (i= 0; i < num_fds; i++)
@@ -2801,7 +2801,7 @@ int main(int argc, char **argv)
 		else
 		{
 			fputs("rsyslogd: Already running.\n", stderr);
-			exit(1);
+			exit(1); /* "good" exit, done if syslogd is already running */
 		}
 	}
 	else
@@ -2822,13 +2822,13 @@ int main(int argc, char **argv)
 			if (!write_pid(PidFile))
 			{
 				dprintf("Can't write pid.\n");
-				exit(1);
+				exit(1); /* exit during startup - questionable */
 			}
 		}
 		else
 		{
 			dprintf("Pidfile (and pid) already exist.\n");
-			exit(1);
+			exit(1); /* exit during startup - questionable */
 		}
 	} /* if ( !Debug ) */
 #endif
@@ -2905,24 +2905,19 @@ int main(int argc, char **argv)
 	/* Create a partial message table for all file descriptors. */
 	num_fds = getdtablesize();
 	dprintf("Allocated parts table for %d file descriptors.\n", num_fds);
-	if ( (parts = (char **) malloc(num_fds * sizeof(char *))) == \
-	    (char **) 0 )
+	if ((parts = (char **) malloc(num_fds * sizeof(char *))) == NULL)
 	{
 		logerror("Cannot allocate memory for message parts table.");
 		die(0);
 	}
 	for(i= 0; i < num_fds; ++i)
-	    parts[i] = (char *) 0;
+	    parts[i] = NULL;
 
 	dprintf("Starting.\n");
 	init();
 #ifndef TESTING
 	if(Debug) {
-		dprintf("Debugging disabled, SIGUSR1 to turn on debugging.\n");
-		/* DEBUG-AID/RELEASE: value 0 below should be used for
-		 * release. Value 1 might be used if you would like to keep debug
-		 * mode enabled during testing. 
-		 */
+		dprintf("Debugging enabled, SIGUSR1 to turn off debugging.\n");
 		debugging_on = 1;
 	}
 	/*
@@ -3227,7 +3222,7 @@ static int usage(void)
 {
 	fprintf(stderr, "usage: rsyslogd [-dhvw] [-l hostlist] [-m markinterval] [-n] [-p path]\n" \
 		" [-s domainlist] [-r port] [-t port] [-f conffile]\n");
-	exit(1);
+	exit(1); /* "good" exit - done to terminate usage() */
 }
 
 #ifdef SYSLOG_UNIXAF
@@ -3249,13 +3244,10 @@ static int create_unix_socket(const char *path)
 	if (fd < 0 || bind(fd, (struct sockaddr *) &sunx,
 			   SUN_LEN(&sunx)) < 0 ||
 	    chmod(path, 0666) < 0) {
-		(void) snprintf(line, sizeof(line), "cannot create %s", path);
+		snprintf(line, sizeof(line), "cannot create %s", path);
 		logerror(line);
 		dprintf("cannot create %s (%d).\n", path, errno);
 		close(fd);
-#ifndef SYSV
-		die(0);
-#endif
 		return -1;
 	}
 	return fd;
@@ -3327,6 +3319,12 @@ static int create_udp_socket()
 }
 #endif
 
+/* rgerhards, 2005-10-24: crunch_list is called only during option processing. So
+ * it is never called once rsyslogd is running (not even when HUPed). This code
+ * contains some exist, but they are considered safe because they only happen
+ * during startup. Anyhow, when we review the code here, we might want to
+ * reconsider the exit()s.
+ */
 static char **crunch_list(char *list)
 {
 	int count, i;
@@ -3352,7 +3350,7 @@ static char **crunch_list(char *list)
 	
 	if ((result = (char **)malloc(sizeof(char *) * (count+2))) == NULL) {
 		printf ("Sorry, can't get enough memory, exiting.\n");
-		exit(0);
+		exit(0); /* safe exit, because only called during startup */
 	}
 	
 	/*
@@ -3365,7 +3363,7 @@ static char **crunch_list(char *list)
 		result[count] = (char *) malloc((q - p + 1) * sizeof(char));
 		if (result[count] == NULL) {
 			printf ("Sorry, can't get enough memory, exiting.\n");
-			exit(0);
+			exit(0); /* safe exit, because only called during startup */
 		}
 		strncpy(result[count], p, q - p);
 		result[count][q - p] = '\0';
@@ -3375,7 +3373,7 @@ static char **crunch_list(char *list)
 	if ((result[count] = \
 	     (char *)malloc(sizeof(char) * strlen(p) + 1)) == NULL) {
 		printf ("Sorry, can't get enough memory, exiting.\n");
-		exit(0);
+		exit(0); /* safe exit, because only called during startup */
 	}
 	strcpy(result[count],p);
 	result[++count] = NULL;
@@ -3389,7 +3387,7 @@ static char **crunch_list(char *list)
 }
 
 
-void untty()
+static void untty()
 #ifdef SYSV
 {
 	if ( !Debug ) {
@@ -3432,7 +3430,6 @@ void untty()
  * no longer rely just on the source (rfc3195d forwarded messages arrive via
  * unix domain sockets but contain the hostname). rgerhards, 2005-10-06
  */
-
 static void printchopped(char *hname, char *msg, int len, int fd, int bParseHost)
 {
 	auto int ptlngth;
@@ -4788,6 +4785,10 @@ static void wallmsg(register struct filed *f)
 	/* TODO: find a way to limit the max size of the message. hint: this
 	 * should go into the template!
 	 */
+	
+	/* rgerhards 2005-10-24: HINT: this code might be run in a seperate thread
+	 * instead of a seperate process once we have multithreading...
+	 */
 
 		/* scan the user login file */
 		while ((uptr = getutent())) {
@@ -4838,7 +4839,7 @@ static void wallmsg(register struct filed *f)
 			}
 			(void) alarm(0);
 		}
-		exit(0);
+		exit(0); /* "good" exit - this terminates the child forked just for message delivery */
 	}
 	/* close the user login file */
 	endutent();
@@ -5024,18 +5025,25 @@ static void logerror(char *type)
 	dprintf("Called logerr, msg: %s\n", type);
 
 	if (errno == 0)
-		(void) snprintf(buf, sizeof(buf), "rsyslogd: %s", type);
+		snprintf(buf, sizeof(buf), "rsyslogd: %s", type);
 	else
-		(void) snprintf(buf, sizeof(buf), "rsyslogd: %s: %s", type, strerror(errno));
+		snprintf(buf, sizeof(buf), "rsyslogd: %s: %s", type, strerror(errno));
 	errno = 0;
 	logmsgInternal(LOG_SYSLOG|LOG_ERR, buf, LocalHostName, ADDDATE);
 	return;
 }
 
-static void die(sig)
 
-	int sig;
-	
+/* die() is called when the program shall end. This typically only occurs
+ * during sigterm or during the initialization. If you search for places where
+ * it is called, search for "die", not "die(", because the later will not find
+ * setting of signal handlers! As die() is intended to shutdown rsyslogd, it is
+ * safe to call exit() here. Just make sure that die() itself is not called
+ * at inapropriate places. As a general rule of thumb, it is a bad idea to add
+ * any calls to die() in new code!
+ * rgerhards, 2005-10-24
+ */
+static void die(int sig)
 {
 	register struct filed *f;
 	char buf[256];
@@ -5137,17 +5145,18 @@ static void die(sig)
 #ifndef TESTING
 	(void) remove_pid(PidFile);
 #endif
-	exit(0);
+	exit(0); /* "good" exit, this is the terminator function for rsyslog [die()] */
 }
 
 /*
  * Signal handler to terminate the parent process.
+ * rgerhards, 2005-10-24: this is only called during forking of the
+ * detached syslogd. I consider this method to be safe.
  */
 #ifndef TESTING
-void doexit(sig)
-	int sig;
+static void doexit(int sig)
 {
-	exit(0);
+	exit(0); /* "good" exit, only during child-creation */
 }
 #endif