[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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()).


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.