Messages in this thread | | | Date | Mon, 21 Jul 2014 09:27:49 -0500 | From | Alex Elder <> | Subject | Re: [PATCH v4 6/7] printk: insert newline for truncated records |
| |
On 07/21/2014 08:57 AM, Petr Mládek wrote: > On Mon 2014-07-21 07:32:18, Alex Elder wrote: >> On 07/21/2014 06:57 AM, Petr Mládek wrote: >>> On Fri 2014-07-18 16:28:04, Alex Elder wrote: >>>> If a log record has LOG_PREFIX set, its predecessor record should be >>>> terminated if it was marked LOG_CONT. >>>> >>>> In devkmsg_read(), this condition was being ignored, which would >>>> lead to such records showing up combined when reading /dev/kmsg. >>>> Fix this oversight.
. . .
>>>> @@ -1023,9 +1029,13 @@ static size_t msg_print_text(const struct printk_log *msg, enum log_flags prev, >>>> >>>> if (buf) { >>>> if (print_prefix(msg, syslog, NULL) + >>>> - text_len + 1 >= size - len) >>>> + text_len + 2 >= size - len) >>> >>> It counts the '\n' even when it is not used. >>> I think that it is even wrong that it calculates prefix when it is not used. >> >> That's true, and I have yet another un-posted patch that >> addresses this problem (well the second one). I am not >> going to fix this problem in this patch, but the fix is >> coming. >> >> Now that you're looking at the code I'm touching, you're >> seeing the same things I did... >> >> I think I'll start posting that series later today or >> tomorrow. I just hate to get too far ahead of myself. > > I suggest to always wait at least 24 hours before sending another > version. I think that it is very hard for others to follow if there > are too many versions in the wild and the code is too changing. > This is why you get comments basically only from me.
Well, what I'm talking about would be a new series. But I'll wait I guess.
FYI, here's what I have already posted: - This series (v5 has now been posted) - A "printk: more log flag simplification" series that follows this one (had problems; will be reposted as a truncated set)
And here are some things I have queued, but have not yet posted for review: - A patch that corrects the size calculation issue you also noticed in msg_print_text(). - A series that I'm still verifying, which avoids formatting all printk() messages repeatedly.
I will try to avoid having too many things out at once.
> Also a night usually helps to sort ideas and go the right direction. > In fact, I was too fast myself as well. I am going to comment only one > version per-day from now on :-)
Oh, all right. :)
>>>> break; >>>> >>>> + if (insert_newline) { >>>> + insert_newline = false; >>>> + buf[len++] = '\n'; >>>> + } >>>> if (prefix) >>>> len += print_prefix(msg, syslog, buf + len); >>>> memcpy(buf + len, text, text_len);
. . .
>> Meanwhile, this patch is addressing essentially the same problem >> in two spots, so I'd like to keep these together rather than >> splitting it in two. > > I do not have strong opinion here.
I will keep it as I posted in v5. I moved this patch earlier in the series, but did not combine it with the first one.
>> I will move this patch earlier in the series, however, making >> it follow patch 1. > > Sounds good.
Thanks for all your timely reviews, Petr.
-Alex -- 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/
| |