This is a simple comma separated list, so use the normal form.

 * Don't cease processing subsequent elements on an error
 * Do report -EINVAL for things like `dom0_nodes=4foo`
 * Don't opencode the cmdline_strcmp() helper

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Jane Malalane <jane.malalane@citrix.com>
---
 xen/arch/x86/dom0_build.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index fe24e11b37fb..5a7441ed5b79 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -169,30 +169,37 @@ bool __initdata dom0_affinity_relaxed;
 
 static int __init parse_dom0_nodes(const char *s)
 {
+    const char *ss;
+    int rc = 0;
+
     do {
+        ss = strchr(s, ',');
+        if ( !ss )
+            ss = strchr(s, '\0');
+
         if ( isdigit(*s) )
         {
+            const char *endp;
+
             if ( dom0_nr_pxms >= ARRAY_SIZE(dom0_pxms) )
-                return -E2BIG;
-            dom0_pxms[dom0_nr_pxms] = simple_strtoul(s, &s, 0);
-            if ( !*s || *s == ',' )
-                ++dom0_nr_pxms;
+                rc = -E2BIG;
+            else if ( (dom0_pxms[dom0_nr_pxms] = simple_strtoul(s, &endp, 0),
+                       endp != ss) )
+                rc = -EINVAL;
+            else
+                dom0_nr_pxms++;
         }
-        else if ( !strncmp(s, "relaxed", 7) && (!s[7] || s[7] == ',') )
-        {
+        else if ( !cmdline_strcmp(s, "relaxed") )
             dom0_affinity_relaxed = true;
-            s += 7;
-        }
-        else if ( !strncmp(s, "strict", 6) && (!s[6] || s[6] == ',') )
-        {
+        else if ( !cmdline_strcmp(s, "strict") )
             dom0_affinity_relaxed = false;
-            s += 6;
-        }
         else
-            return -EINVAL;
-    } while ( *s++ == ',' );
+            rc = -EINVAL;
 
-    return s[-1] ? -EINVAL : 0;
+        s = ss + 1;
+    } while ( *ss );
+
+    return rc;
 }
 custom_param("dom0_nodes", parse_dom0_nodes);
 
-- 
2.11.0

On 19.11.2021 14:44, Andrew Cooper wrote:
> This is a simple comma separated list, so use the normal form.
> 
>  * Don't cease processing subsequent elements on an error
>  * Do report -EINVAL for things like `dom0_nodes=4foo`
>  * Don't opencode the cmdline_strcmp() helper
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

I guess you will want this backported?

Jan

On 19/11/2021 14:01, Jan Beulich wrote:
> On 19.11.2021 14:44, Andrew Cooper wrote:
>> This is a simple comma separated list, so use the normal form.
>>
>>  * Don't cease processing subsequent elements on an error
>>  * Do report -EINVAL for things like `dom0_nodes=4foo`
>>  * Don't opencode the cmdline_strcmp() helper
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> I guess you will want this backported?

In due course, probably.

For now, Jane is investigating why dom0_nodes=0 isn't working, and this
was a tiny bit I spotted on the side and just decided to fix.

I'm fairly confident there will be more bugfixes coming.

~Andrew