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

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



Hi,

On 05/12/2019 15:33, Jan Beulich wrote:
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>

--- 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,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.

+{
+    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 _?

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.

+{
+    int res;
+
+    for ( ; ; ++s1, ++s2 )
+    {
+        res = cdiff(*s1, *s2);
+        if ( res || !*s1 )
+            break;
+    }
+
+    return res;
+}
+
+static int _strncmp(const char *s1, const char *s2, size_t n)
+{
+    int res = 0;
+
+    for ( ; n--; ++s1, ++s2 )
+    {
+        res = cdiff(*s1, *s2);
+        if ( res || !*s1 )
+            break;
+    }
+
+    return res;
+}
+
  static int assign_integer_param(const struct kernel_param *param, uint64_t 
val)
  {
      switch ( param->len )
@@ -94,7 +137,7 @@ static int parse_params(const char *cmdl
/* Boolean parameters can be inverted with 'no-' prefix. */
          key = optkey;
-        bool_assert = !!strncmp("no-", optkey, 3);
+        bool_assert = !!_strncmp("no-", optkey, 3);
          if ( !bool_assert )
              optkey += 3;
@@ -105,11 +148,11 @@ static int parse_params(const char *cmdl
              int rctmp;
              const char *s;
- if ( strcmp(param->name, optkey) )
+            if ( _strcmp(param->name, optkey) )
              {
                  if ( param->type == OPT_CUSTOM && q &&
                       strlen(param->name) == q + 1 - opt &&
-                     !strncmp(param->name, opt, q + 1 - opt) )
+                     !_strncmp(param->name, opt, q + 1 - opt) )
                  {
                      found = true;
                      optval[-1] = '=';
@@ -275,7 +318,7 @@ int parse_bool(const char *s, const char
  int parse_boolean(const char *name, const char *s, const char *e)
  {
      size_t slen, nlen;
-    int val = !!strncmp(s, "no-", 3);
+    int val = !!_strncmp(s, "no-", 3);
if ( !val )
          s += 3;
@@ -284,7 +327,7 @@ int parse_boolean(const char *name, cons
      nlen = strlen(name);
/* Does s now start with name? */
-    if ( slen < nlen || strncmp(s, name, nlen) )
+    if ( slen < nlen || _strncmp(s, name, nlen) )
          return -1;
/* Exact, unadorned name? Result depends on the 'no-' prefix. */
@@ -304,7 +347,7 @@ int cmdline_strcmp(const char *frag, con
      for ( ; ; frag++, name++ )
      {
          unsigned char f = *frag, n = *name;
-        int res = f - n;
+        int res = cdiff(f, n);
if ( res || n == '\0' )
          {


--
Julien Grall

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

 


Rackspace

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