[PATCH]Avoid klogd splitting lines

Denys Vlasenko vda.linux at googlemail.com
Sat Oct 4 15:22:00 UTC 2008


On Thursday 02 October 2008 03:16:42 am Steve Bennett wrote:
> klogd.c says:
> 
> > -   /* Note: this code does not detect incomplete messages
> > -    * (messages not ending with '\n' or just when kernel
> > -    * generates too many messages for us to keep up)
> > -    * and will split them in two separate lines */
> 
> This is particularly noticeable at startup where there are many
> initial messages and lots of them are split.
> 
> The attached patch fixes this at the cost of a few bytes.
> 
> function                                             old     new   delta
> klogd_main                                           362     406     +44
> ------------------------------------------------------------------------------
> (add/remove: 0/0 grow/shrink: 1/0 up/down: 44/0)               Total:  
> 44 bytes
> 
> Please consider applying.

                n = klogctl(2, log_buffer + used, KLOGD_LOGBUF_SIZE - used - 1);
                if (n < 0) { ...
                        break;
                }
                /* klogctl buffer parsing modelled after code in dmesg.c */
                start = &log_buffer[0];
                /* Process each newline-terminated line in the buffer */
                while (1) {
                        char *newline = memchr(start, '\n', n);

Bug. n is wrong, you have to adjust it (for example, by n += used).

                        if (*start) {
                                syslog(priority, "%s", start);
You output the string starting from start...
                        }
                        if (newline) {
                                start = newline;
                        }
                        else {
                                if (start != log_buffer) {
                                        /* This line is incomplete, so move it to the front of the buffer */
                                        memmove(log_buffer, start, n);
                                        used = n;
                                }
                                break;

...but you do not discard it, you move it to the beginning
of the buffer. This will print incomplete line again
on next iteration.

Please test this one.
--
vda


diff -d -urpN busybox.8/sysklogd/klogd.c busybox.9/sysklogd/klogd.c
--- busybox.8/sysklogd/klogd.c	2008-09-28 17:02:08.000000000 +0200
+++ busybox.9/sysklogd/klogd.c	2008-10-04 17:11:44.000000000 +0200
@@ -44,6 +44,8 @@ int klogd_main(int argc UNUSED_PARAM, ch
 	int i = 0;
 	char *start;
 	int opt;
+	int priority;
+	int used = 0;
 
 	opt = getopt32(argv, "c:n", &start);
 	if (opt & OPT_LEVEL) {
@@ -72,16 +74,14 @@ int klogd_main(int argc UNUSED_PARAM, ch
 
 	syslog(LOG_NOTICE, "klogd started: %s", bb_banner);
 
-	/* Note: this code does not detect incomplete messages
-	 * (messages not ending with '\n' or just when kernel
-	 * generates too many messages for us to keep up)
-	 * and will split them in two separate lines */
+	/* Initially null terminate the buffer in case of a very long line */
+	log_buffer[KLOGD_LOGBUF_SIZE - 1] = '\0';
+
 	while (1) {
 		int n;
-		int priority;
 
 		/* "2 -- Read from the log." */
-		n = klogctl(2, log_buffer, KLOGD_LOGBUF_SIZE - 1);
+		n = klogctl(2, log_buffer + used, KLOGD_LOGBUF_SIZE-1 - used);
 		if (n < 0) {
 			if (errno == EINTR)
 				continue;
@@ -89,32 +89,47 @@ int klogd_main(int argc UNUSED_PARAM, ch
 					errno);
 			break;
 		}
-		log_buffer[n] = '\n';
-		i = 0;
-		while (i < n) {
+
+		/* klogctl buffer parsing modelled after code in dmesg.c */
+		start = &log_buffer[0];
+
+		/* Process each newline-terminated line in the buffer */
+		while (1) {
+			char *newline = strchr(start, '\n');
+
+			if (!newline) {
+				/* This line is incomplete... */
+				if (start != log_buffer) {
+					/* move it to the front of the buffer */
+					strcpy(log_buffer, start);
+					/* don't log it yet */
+					used = strlen(log_buffer);
+					break;
+				}
+				/* ...but buffer is full, so log it anyway */
+				used = 0;
+			} else {
+				*newline++ = '\0';
+			}
+
+			/* Extract the priority */
 			priority = LOG_INFO;
-			start = &log_buffer[i];
-			if (log_buffer[i] == '<') {
-				i++;
-				// kernel never ganerates multi-digit prios
-				//priority = 0;
-				//while (log_buffer[i] >= '0' && log_buffer[i] <= '9') {
-				//	priority = priority * 10 + (log_buffer[i] - '0');
-				//	i++;
-				//}
-				if (isdigit(log_buffer[i])) {
-					priority = (log_buffer[i] - '0');
-					i++;
+			if (*start == '<') {
+				start++;
+				if (*start) {
+					/* kernel never generates multi-digit prios */
+					priority = (*start - '0');
+					start++;
+				}
+				if (*start == '>') {
+					start++;
 				}
-				if (log_buffer[i] == '>')
-					i++;
-				start = &log_buffer[i];
 			}
-			while (log_buffer[i] != '\n')
-				i++;
-			log_buffer[i] = '\0';
-			syslog(priority, "%s", start);
-			i++;
+			if (*start)
+				syslog(priority, "%s", start);
+			if (!newline)
+				break;
+			start = newline;
 		}
 	}
 



More information about the busybox mailing list