[PATCH]Avoid klogd splitting lines

Steve Bennett steveb at workware.net.au
Mon Oct 6 06:45:04 UTC 2008


Hi Denys,

You're right. That's what I get for make some last minute
tweaks before sending :-(

Your patch works nicely.

Cheers,
Steve

On 05/10/2008, at 1:22 AM, Denys Vlasenko wrote:

> 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;
> 		}
> 	}
>
>

--
WorkWare Systems Pty Ltd
W: www.workware.net.au      P: 0434 921 300
E: steveb at workware.net.au   F: 07 3102 9221






More information about the busybox mailing list