Re: [PATCH v2 8/9] lib/vsprintf: Remove useless NULL checks

[Petr Mladek]

On Wed 2018-02-28 12:42:24, Andy Shevchenko wrote:
> On Wed, 2018-02-28 at 11:04 +0100, Petr Mladek wrote:
> > On Tue 2018-02-27 19:35:50, Andy Shevchenko wrote:
> > > On Tue, 2018-02-27 at 16:50 +0100, Petr Mladek wrote:
> > Note that it will most likely crash in vprintk_emit() on the line
> > 
> >    text_len = vscnprintf(text, sizeof(textbuf), fmt, args);
> > 
> > It will be with logbug_lock() taken. The nested printk() messages
> > will be stored in per-CPU buffer thanks to printk_safe code.
> 
> Yeah, that's bad.
> 
> > IMHO, it would make sense to hanve this check also pointers that are
> > being deferred.
> 
> Send a patch to discuss!

I thought about this more. IMHO, the check for PAGE_SIZE in pointer()
makes perfect sense. It helps to avoid crash and actually see
the message. I am going to send the patch in a minute.

BTW: I am not sure who is going to pass this patchset to Linus.
If nobody is against, I could eventually do so via printk.git.


> > > > To be honest, I do not feel experienced enough to decide
> > > > about the preferred behavior. On one hand, it is bad when
> > > > printk() would crash the kernel. On the other hand, hiding wide
> > > > range of values under "(null)" string might confuse people.
> > > > Would it make sense to survive and write different strings for
> > > > difference intervals? For example?
> > > > 
> > > >     "(null)"     for ptr == 0
> > > >     "(null-16)"  for ptr > 0 && ptr <= 16
> > > >     "(null-pg)"  for prt > 16 && ptr <= PAGE_SIZE
> > > > 
> > > > In each case, this patch changes the behavior and it should
> > > > be documented in the commit message.
> > > 
> > > Personally I strongly disagree with blowing code up in such places
> > > for little or none benefit.
> 
> Send a patch to discuss!

I am not going to do so unless there is an evidence that people are
confused or that the above idea is desired.

Best Regards,
Petr