Re: [PATCH] printk.c: removed unnecessary code

[Petr Mladek]

On Wed 2016-11-30 10:14:28, Michal Hocko wrote:
> [Resending with the full CC list as my email client has clobbered it in
> the previous attempt for some reason]
> 
> On Tue 29-11-16 16:19:01, Sebastian Duda wrote:
> > snprintf((char *) ?, 0, ...); always returns Zero and doesn't change the data.
> > Thus the execution of
> > 	snprintf(NULL, 0, "[%5lu.000000] ", (unsigned long)ts);
> > has no effect on program.
> > The substitution with 0 increases the readability of the code.
> 
> Are you sure this is correct. As per vsnprintf documentation:
> "
>  * The return value is the number of characters which would
>  * be generated for the given input, excluding the trailing
>  * '\0', as per ISO C99.
> "
> 
> this should just work as 35dac27cedd1 ("printk: fix incorrect length
> from print_time() when seconds > 99999") intended.
> 
> I haven't checked the implementation though so I might be wrong here.

print_time() actually is not used by printk() directly. Therefore
the above reasoning does not apply.

They key here is to look when the buf might be NULL. It is when
we try to get an idea how much space will be needed for the message
when it is printed to some output device, e.g. console or syslog.
For example, there is the following chain of calls:

  syslog_print_all()
     msg_print_text(msg, prev, true, NULL, 0)
       print_prefix(msg, syslog, NULL)
         len += print_time(msg->ts_nsec, buf ? buf + len : NULL)

It means that we really need to know the length of the printed
string even when we do not write it to a buffer for the moment.

By other words, the patch would break the functionality.

> > 
> > Signed-off-by: Sebastian Duda <sebastian.duda@xxxxxx>
> > Signed-off-by: Tobias Baumeister <tobias.baumeister@xxxxxx>

Nacked-by: Petr Mladek <pmladek@xxxxxxxx>

Best Regards,
Petr