Re: [Xen-devel] [PATCH] console: avoid printing no or null time stamps

[Doug Goldstein <cardoe@xxxxxxxxxx>]

On Mon, Jul 02, 2018 at 02:47:42PM +0100, Julien Grall wrote:
> On 06/26/2018 10:03 AM, Jan Beulich wrote:
> > > > > On 26.06.18 at 10:43, <julien.grall@xxxxxxx> wrote:
> > > On 26/06/18 08:24, Jan Beulich wrote:
> > > > @@ -698,26 +701,30 @@ static void printk_start_of_line(const c
> > > >        case TSM_DATE_MS:
> > > >            tm = wallclock_time(&nsec);
> > > > -        if ( tm.tm_mday == 0 )
> > > > -            return;
> > > > -
> > > > -        if ( opt_con_timestamp_mode == TSM_DATE )
> > > > -            snprintf(tstr, sizeof(tstr), "[%04u-%02u-%02u 
> > > > %02u:%02u:%02u] ",
> > > > -                     1900 + tm.tm_year, tm.tm_mon + 1, tm.tm_mday,
> > > > -                     tm.tm_hour, tm.tm_min, tm.tm_sec);
> > > > -        else
> > > > +        if ( tm.tm_mday )
> > > > +        {
> > > >                snprintf(tstr, sizeof(tstr),
> > > > -                     "[%04u-%02u-%02u %02u:%02u:%02u.%03"PRIu64"] ",
> > > > +                     opt_con_timestamp_mode == TSM_DATE
> > > > +                     ? "[%04u-%02u-%02u %02u:%02u:%02u] "
> > > > +                     : "[%04u-%02u-%02u %02u:%02u:%02u.%03"PRIu64"] ",
> > > >                         1900 + tm.tm_year, tm.tm_mon + 1, tm.tm_mday,
> > > >                         tm.tm_hour, tm.tm_min, tm.tm_sec, nsec / 
> > > > 1000000);
> > > 
> > > I find this change rather difficult to read because the number of
> > > arguments for the 2 formats are different. It would be better to keep
> > > the two snprintf separately.
> > 
> > And I find the redundancy rather ugly to maintain, so I'd prefer to stick to
> > single invocation.
> 
> Maybe it is for you. Not for me.

I'm in agreement with Julien. Its not easy to follow and certainly not
having the number of arguments line up looks like a "code smell" to me.

--
Doug

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel