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

Re: [Xen-devel] [PATCH v2 1/2] cmdline_parse: Convert no- prefix into =no for OPT_CUSTOM




On 07/30/14 03:58, Jan Beulich wrote:
On 29.07.14 at 21:57, <dslutz@xxxxxxxxxxx> wrote:
This allows converting OPT_BOOL into OPT_CUSTOM and still do what is
expected.

Note: if both no- and = provided, pass provided = if not a zero
length string.  Also log a warning about this.

Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
---
  xen/common/kernel.c | 15 ++++++++++++++-
  1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 7e83353..9857159 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -135,7 +135,20 @@ void __init cmdline_parse(const char *cmdline)
                      parse_size_and_unit(optval, NULL));
                  break;
              case OPT_CUSTOM:
-                ((void (*)(const char *))param->var)(optval);
+                if ( !bool_assert )
+                {
+                    if ( *optval )
+                    {
+                        printk(XENLOG_WARNING
+                               "Warning: ignored no- prefix for %s=%s\n",
+                               param->name, optval);
Log messages printed before the consoles get initialized are of limited
value (they would only show up in "xl dmesg" or equivalent output).
Therefore I'm not really certain this is worthwhile.

I am happy to go either way.

+                        ((void (*)(const char *))param->var)(optval);
+                    }
+                    else
+                        ((void (*)(const char *))param->var)("no");
+                }
+                else
+                    ((void (*)(const char *))param->var)(optval);
Considering this repeated cast I'd prefer coding this quite
differently, not the least because you also can't pass a literal
"no" to the parsing functions (as they're permitted to alter the
string), and at once also ignoring bogus "no-<name>=<value>"
options instead of trying to assign a (perhaps unexpected)
meaning:

convert "no-" command line option prefix into "=no" for OPT_CUSTOM

... to allow restoring/retaining previous behavior for options getting
converted from boolean to custom.

Reported-by: Don Slutz <dslutz@xxxxxxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -135,6 +135,13 @@ void __init cmdline_parse(const char *cm
                      parse_size_and_unit(optval, NULL));
                  break;
              case OPT_CUSTOM:
+                if ( !bool_assert )
+                {
+                    if ( *optval )
+                        break;
+                    safe_strcpy(opt, "no");
+                    optval = opt;
+                }
                  ((void (*)(const char *))param->var)(optval);
                  break;
              default:



Without the log message, I think it might help to include the part about
ignoring bogus "no-<name>=<value>"... aka stacked inversions in the
commit message.  This is because boolean options do still support stacked
inversions, which is valid under the statement "Explicitly specifying any
value other than those listed above is undefined, as is stacking a `no-`
prefix with an explicit value."

I am happy with the actual code change.

   -Don Slutz



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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