On 10/08/2018 09:08, Jan Beulich wrote: >>>> On 09.08.18 at 18:38, <andrew.coop...@citrix.com> 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. 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. 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... ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
