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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
