On 2020-02-14, Petr Mladek <pmladek@xxxxxxxx> wrote: >> I oversaw that devkmsg_open() setup a printk_record and so I did not >> see to add the extra NULL initialization of text_line_count. There >> should be be an initializer function/macro to avoid this danger. >> >> John Ogness >> >> The quick fixup: >> >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c >> index d0d24ee1d1f4..5ad67ff60cd9 100644 >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -883,6 +883,7 @@ static int devkmsg_open(struct inode *inode, struct file *file) >> user->record.text_buf_size = sizeof(user->text_buf); >> user->record.dict_buf = &user->dict_buf[0]; >> user->record.dict_buf_size = sizeof(user->dict_buf); >> + user->record.text_line_count = NULL; > > The NULL pointer hidden in the structure also complicates the code > reading. It is less obvious when the same function is called > only to get the size/count and when real data. OK. > I played with it and created extra function to get this information. > > In addition, I had problems to follow the code in > record_print_text_inline(). So I tried to reuse the new function > and the existing record_printk_text() there. > > Please, find below a patch that I ended with. I booted a system > with this patch. But I guess that I actually did not use the > record_print_text_inline(). So, it might be buggy. Yes, there are several bugs. But I see where you want to go with this: - introduce prb_count_lines() to handle line counting - introduce prb_read_valid_info() for only reading meta-data and getting the line count - also use prb_count_lines() internally I will include these changes in v2. I will still introduce the static inlines to initialize records because readers and writers do it differently. Thanks. John Ogness _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
