Messages in this thread | | | Date | Thu, 01 Jun 2006 10:40:32 +0200 | From | Martin Peschke <> | Subject | Re: [Patch 3/6] statistics infrastructure - prerequisite: timestamp |
| |
Tim Bird wrote: > Martin Peschke wrote: >> And I think that I have fixed a printed_len related miscalculation. >> printed_len needs to be increased if no valid log level has been found >> and a log level prefix has been added by printk(). Otherwise, printed_len >> must not be increased. The old code did it the other way around (in the >> timestamp case). > > I'm not following your change here. I can't find the problem you > mention in the original code. And your fix appears to mess up the > printed_len.
I am sorry for not responding sooner ... and I am sorry for introducing a bug here. I think you are right.
>> + if (new_line) { >> + /* The log level token is first. */ >> + int loglev_char; >> + if (p[0] == '<' && p[1] >='0' && >> + p[1] <= '7' && p[2] == '>') { >> + loglev_char = p[1]; >> + p += 3; >> + printed_len -= 3; > Here you subtract from the printed_len to account for skipping > the submitted log level. > >> + } else { >> + loglev_char = default_message_loglevel + '0'; >> + } >> + emit_log_char('<'); >> + emit_log_char(loglev_char); >> + emit_log_char('>'); > > But here you don't re-count the chars for the log-level > you are adding back in. There's a discrepancy here.
Correct. My print_len loses 3 in the 'got log level'-case due to a surplus substraction. It also loses 3 in the other case due to adding a log level substring that is not entered in the books.
This is how I would fix it:
printed_len = vscnprintf(printk_buf, sizeof(printk_buf), fmt, args);
...
if (new_line) { /* The log level token is first. */ int loglev_char; if (p[0] == '<' && p[1] >='0' && p[1] <= '7' && p[2] == '>') { loglev_char = p[1]; p += 3; } else { loglev_char = default_message_loglevel + '0'; print_len += 3; } emit_log_char('<'); emit_log_char(loglev_char); emit_log_char('>');
Could you confirm, please. I will send a patch to Andrew then.
Or, Andrew, do you prefer a replacement patch for my statistics-infrastructure-prerequisite-timestamp.patch that introduces this miscalculation. I could put statistics-infrastructure-make-printk_clock-a-generic-kernel-wide-nsec-resolution.patch into that one as well, so that all the timestamp related printk-changes are in one place.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |