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

[George Dunlap <george.dunlap@citrix.com>]

On 12/6/19 2:46 PM, Julien Grall wrote:
> Hi Jan,
> 
> On 05/12/2019 16:50, Jan Beulich wrote:
>> 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.
> 
> I am not sure to understand why non-static symbols only. This would
> prevent you to use the the non-static symbol if you happen to re-use the
> same name.
> 
> Anyway, how about calling it cmdline_strncmp()? This would be easier to
> spot misuse on review (i.e using strncmp() rather than _strncmp()).

FWIW I was thinking something similar.

 -George

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