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

From: Jan Beulich <jbeulich@suse.com>
To: Julien Grall <julien@xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Konrad Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH] cmdline: treat hyphens and underscores the same
Date: Thu, 5 Dec 2019 17:50:59 +0100	[thread overview]
Message-ID: <271f8148-e74a-3d3b-375d-991d7cb9e475@suse.com> (raw)
In-Reply-To: <8dc4166d-45c3-0a5c-8782-78de1b74ad36@xen.org>

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel