Re: [PATCH v7 1/2] x86/time: introduce command line option to select wallclock

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Sep 13, 2024 at 02:38:14PM +0200, Jan Beulich wrote:
> On 13.09.2024 09:59, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/time.c
> > +++ b/xen/arch/x86/time.c
> > @@ -1552,6 +1552,37 @@ static const char *__init 
> > wallclock_type_to_string(void)
> >      return "";
> >  }
> >  
> > +static int __init cf_check parse_wallclock(const char *arg)
> > +{
> > +    wallclock_source = WALLCLOCK_UNSET;
> 
> With this ...
> 
> > +    if ( !arg )
> > +        return -EINVAL;
> > +
> > +    if ( !strcmp("auto", arg) )
> > +        ASSERT(wallclock_source == WALLCLOCK_UNSET);
> 
> ... I'm not convinced this is (still) needed.

It reduces to an empty statement in release builds, and is IMO clearer
code wise.  I could replace the assert with a comment, but IMO the
assert conveys the same information in a more compact format and
provides extra safety in case the code is changed and wallclock_source
is no longer initialized to the expected value.

Thanks, Roger.