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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.12.2019 17:27, Julien Grall wrote:
> On 05/12/2019 15:33, Jan Beulich wrote:
>> --- a/xen/common/kernel.c
>> +++ b/xen/common/kernel.c
>> @@ -23,6 +23,49 @@ enum system_state system_state = SYS_STA
>>   xen_commandline_t saved_cmdline;
>>   static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
>>   
>> +static int cdiff(unsigned char c1, unsigned char c2)
> 
> This is not obvious from the name and the implementation what it does 
> (it took me a few minutes to figure it out). So I think you want to add 
> a comment.

Sure, done.

>> +{
>> +    int res = c1 - c2;
>> +
>> +    if ( res && (c1 ^ c2) == ('-' ^ '_') &&
>> +         (c1 == '-' || c1 == '_') )
>> +        res = 0;
>> +
>> +    return res;
>> +}
>> +
>> +/*
>> + * String comparison functions mostly matching strcmp() / strncmp(),
>> + * except that they treat '-' and '_' as matching one another.
>> + */
>> +static int _strcmp(const char *s1, const char *s2)
> 
> I thought we were trying to avoid new function name with leading _?

We're trying to avoid new name space violations. Such are
- identifiers starting with two underscores,
- identifiers starting with an underscore and an upper case letter,
- identifiers of non-static symbols starting with an underscore.

> But it is really worth to implement both strcmp and strncmp rather than 
> using the latter to implement the former?
> 
> I know this involve using strlen, but I am not convinced this will be 
> noticeable at boot.

To be honest - it didn't even occur to me. The functions seemed
simple enough to not have one use the other. If others agree
with you, I'd be fine doing as you suggest.

Jan

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