xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the same
@ 2020-01-23 11:42 Jan Beulich
  2020-01-23 12:11 ` Durrant, Paul
  2020-01-23 16:19 ` Andrew Cooper
  0 siblings, 2 replies; 4+ messages in thread
From: Jan Beulich @ 2020-01-23 11:42 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson

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@suse.com>
---
v2: Rename to opt_str{,n}cmp(). Don't use the new function for comapring
    against "no-" in parse_params(). Add comment to cdiff().

--- 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,53 @@ enum system_state system_state = SYS_STA
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
+/*
+ * Calculate the difference between two characters for command line parsing
+ * purposes, i.e. treating '-' and '_' the same.
+ */
+static int cdiff(unsigned char c1, unsigned char c2)
+{
+    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 opt_strcmp(const char *s1, const char *s2)
+{
+    int res;
+
+    for ( ; ; ++s1, ++s2 )
+    {
+        res = cdiff(*s1, *s2);
+        if ( res || !*s1 )
+            break;
+    }
+
+    return res;
+}
+
+static int opt_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 +141,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 = !!opt_strncmp("no-", optkey, 3);
         if ( !bool_assert )
             optkey += 3;
 
@@ -105,11 +152,11 @@ static int parse_params(const char *cmdl
             int rctmp;
             const char *s;
 
-            if ( strcmp(param->name, optkey) )
+            if ( opt_strcmp(param->name, optkey) )
             {
                 if ( param->type == OPT_CUSTOM && q &&
                      strlen(param->name) == q + 1 - opt &&
-                     !strncmp(param->name, opt, q + 1 - opt) )
+                     !opt_strncmp(param->name, opt, q + 1 - opt) )
                 {
                     found = true;
                     optval[-1] = '=';
@@ -284,7 +331,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 || opt_strncmp(s, name, nlen) )
         return -1;
 
     /* Exact, unadorned name?  Result depends on the 'no-' prefix. */
@@ -304,7 +351,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' )
         {

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the same
  2020-01-23 11:42 [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the same Jan Beulich
@ 2020-01-23 12:11 ` Durrant, Paul
  2020-01-23 13:27   ` Jan Beulich
  2020-01-23 16:19 ` Andrew Cooper
  1 sibling, 1 reply; 4+ messages in thread
From: Durrant, Paul @ 2020-01-23 12:11 UTC (permalink / raw)
  To: Jan Beulich, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan
> Beulich
> Sent: 23 January 2020 11:43
> To: xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien@xen.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>
> Subject: [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the
> same
> 
> 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@suse.com>
> ---
> v2: Rename to opt_str{,n}cmp(). Don't use the new function for comapring
>     against "no-" in parse_params(). Add comment to cdiff().
> 
> --- 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,53 @@ enum system_state system_state = SYS_STA
>  xen_commandline_t saved_cmdline;
>  static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
> 
> +/*
> + * Calculate the difference between two characters for command line
> parsing
> + * purposes, i.e. treating '-' and '_' the same.
> + */
> +static int cdiff(unsigned char c1, unsigned char c2)
> +{
> +    int res = c1 - c2;
> +
> +    if ( res && (c1 ^ c2) == ('-' ^ '_') &&
> +         (c1 == '-' || c1 == '_') )
> +        res = 0;
> +

Wow! That makes my head hurt. How about:

static int cdiff(unsigned char c1, unsigned char c2)
{
    if ( c1 == '-' )
        c1 = '_';

    if ( c2 == '-' )
        c2 = '_';

    return c1 - c2;
}

?

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the same
  2020-01-23 12:11 ` Durrant, Paul
@ 2020-01-23 13:27   ` Jan Beulich
  0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2020-01-23 13:27 UTC (permalink / raw)
  To: Durrant, Paul
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel

On 23.01.2020 13:11, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan
>> Beulich
>> Sent: 23 January 2020 11:43
>> To: xen-devel@lists.xenproject.org
>> Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
>> <julien@xen.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>
>> Subject: [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the
>> same
>>
>> 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@suse.com>
>> ---
>> v2: Rename to opt_str{,n}cmp(). Don't use the new function for comapring
>>     against "no-" in parse_params(). Add comment to cdiff().
>>
>> --- 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,53 @@ enum system_state system_state = SYS_STA
>>  xen_commandline_t saved_cmdline;
>>  static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
>>
>> +/*
>> + * Calculate the difference between two characters for command line
>> parsing
>> + * purposes, i.e. treating '-' and '_' the same.
>> + */
>> +static int cdiff(unsigned char c1, unsigned char c2)
>> +{
>> +    int res = c1 - c2;
>> +
>> +    if ( res && (c1 ^ c2) == ('-' ^ '_') &&
>> +         (c1 == '-' || c1 == '_') )
>> +        res = 0;
>> +
> 
> Wow! That makes my head hurt. How about:
> 
> static int cdiff(unsigned char c1, unsigned char c2)
> {
>     if ( c1 == '-' )
>         c1 = '_';
> 
>     if ( c2 == '-' )
>         c2 = '_';
> 
>     return c1 - c2;
> }
> 
> ?

This would work for the current uses where ultimately the
result is only evaluated for being (non-)zero, but wouldn't
be correct when used for actual collation.

Jan

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the same
  2020-01-23 11:42 [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the same Jan Beulich
  2020-01-23 12:11 ` Durrant, Paul
@ 2020-01-23 16:19 ` Andrew Cooper
  1 sibling, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2020-01-23 16:19 UTC (permalink / raw)
  To: Jan Beulich, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Wilk,
	George Dunlap, Ian Jackson

On 23/01/2020 11:42, 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@suse.com>
> ---
> v2: Rename to opt_str{,n}cmp(). Don't use the new function for comapring
>     against "no-" in parse_params(). Add comment to cdiff().

Personally, I think this is a bad idea and should not be continued.

Yes - it is irritating needing to remember whether an option is spelled
with hyphen or underscore, but that is nothing compared to the pain for
users, who will have less bleeding edge version of Xen where the
different really matters.

Having one consistent behaviour across all versions of Xen is of far
more value to people than trying to fix a few wonky corner cases for
developers benefit.

~Andrew

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-23 16:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-23 11:42 [Xen-devel] [PATCH v2] cmdline: treat hyphens and underscores the same Jan Beulich
2020-01-23 12:11 ` Durrant, Paul
2020-01-23 13:27   ` Jan Beulich
2020-01-23 16:19 ` Andrew Cooper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).