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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.09.2024 09:50, Roger Pau Monné wrote:
> 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.

Well, so be it then.

Jan