Re: [Xen-devel] [PATCH] x86/spec-ctrl: Yet more fixes for xpti= parsing

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.08.18 at 11:49, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 10/08/2018 09:08, Jan Beulich wrote:
>>>>> On 09.08.18 at 18:38, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> As it currently stands, 'xpti=dom0' is indistinguishable from the default
>>> value, which means it will be overridden by ARCH_CAPABILITIES_RDCL_NO on 
> fixed
>>> hardware.
>>>
>>> Switch opt_xpti to use -1 as a default like all our other related options, 
> and
>>> clobber it as soon as we have a string to parse.
>>>
>>> In addition, 'xpti' alone should be interpreted in its positive boolean 
> form,
>>> rather than resulting in a parse error.
>> But e.g. "xpti=dom0," should not be. I.e. ...
>>
>>> @@ -439,6 +438,10 @@ static __init int parse_xpti(const char *s)
>>>      const char *ss;
>>>      int val, rc = 0;
>>>  
>>> +    /* Inhibit the defaults as an explicit choice has been given. */
>>> +    if ( opt_xpti == -1 )
>>> +        opt_xpti = 0;
>> ... the check for an empty string pointed to by s needs to be put here,
>> ahead of the loop.
> 
> There are 3 options:
> 
> 1) What is presented here.
> 2) Unroll the start of the loop to be able to reach case 1:
> 3) Double up the positive case.
> 
> Given how you review code generally, options 2 and 3 are off the table.

Hmm, 2 certainly is, but 3 seems reasonable to me.

> If someone typo's the command like, nothing is going to work as they
> intended in the first place.  Therefore, "xpti=dom0," doing the wrong
> thing is not a problem.

But "xpti=dom0," meaning "xpti=dom0" is far more expectable
than is meaning "xpti=dom0,true". The only viable option besides
ignoring the trailing comma would seem to be to consider the
entire option invalid in such a case, but ignoring is significantly
easier to arrange for.

> I don't see why we should go out of our way to cover that specific
> corner case, or do you suggest we start putting a spellchecker into the
> parsing and start guessing at what the user meant?  This would be far
> more useful if you want to start second guessing what the user wrote...

I don't mean us to second guess anything.

Jan



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