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: [EMAIL PROTECTED] F: 07 3102 9221
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
