Re: [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the same

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.01.2020 13:11, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan
>> Beulich
>> Sent: 23 January 2020 11:43
>> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall
>> <julien@xxxxxxx>; Wei Liu <wl@xxxxxxx>; Konrad Wilk
>> <konrad.wilk@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxxxxx>;
>> Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Ian Jackson
>> <ian.jackson@xxxxxxxxxx>
>> Subject: [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the
>> same
>>
>> In order to avoid permanently having to ask that no new command line
>> options using underscores be introduced (albeit I'm likely to still make
>> remarks), and in order to also allow extending the use of hyphens to
>> pre-existing ones, introduce custom comparison functions treating both
>> characters as matching.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> v2: Rename to opt_str{,n}cmp(). Don't use the new function for comapring
>>     against "no-" in parse_params(). Add comment to cdiff().
>>
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -72,6 +72,11 @@ Some options take a comma separated list
>>  Some parameters act as combinations of the above, most commonly a mix
>>  of Boolean and String.  These are noted in the relevant sections.
>>
>> +### Spelling
>> +
>> +Parameter names may include hyphens or underscores.  These are
>> +generally being treated as matching one another by the parsing logic.
>> +
>>  ## Parameter details
>>
>>  ### acpi
>> --- a/xen/common/kernel.c
>> +++ b/xen/common/kernel.c
>> @@ -23,6 +23,53 @@ enum system_state system_state = SYS_STA
>>  xen_commandline_t saved_cmdline;
>>  static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
>>
>> +/*
>> + * Calculate the difference between two characters for command line
>> parsing
>> + * purposes, i.e. treating '-' and '_' the same.
>> + */
>> +static int cdiff(unsigned char c1, unsigned char c2)
>> +{
>> +    int res = c1 - c2;
>> +
>> +    if ( res && (c1 ^ c2) == ('-' ^ '_') &&
>> +         (c1 == '-' || c1 == '_') )
>> +        res = 0;
>> +
> 
> Wow! That makes my head hurt. How about:
> 
> static int cdiff(unsigned char c1, unsigned char c2)
> {
>     if ( c1 == '-' )
>         c1 = '_';
> 
>     if ( c2 == '-' )
>         c2 = '_';
> 
>     return c1 - c2;
> }
> 
> ?

This would work for the current uses where ultimately the
result is only evaluated for being (non-)zero, but wouldn't
be correct when used for actual collation.

Jan

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