diff mbox

[v2,18/52] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

Message ID 20170814070849.20986-19-jgross@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jürgen Groß Aug. 14, 2017, 7:08 a.m. UTC 
Modify the custom parameter parsing routines in:

xen/arch/x86/psr.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/psr.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

Comments

Jan Beulich Aug. 14, 2017, 1:35 p.m. UTC | #1 
>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -420,7 +420,7 @@ static const struct feat_props l2_cat_props = {
>  };
>  
>  static void __init parse_psr_bool(char *s, char *value, char *feature,
> -                                  unsigned int mask)
> +                                  unsigned int mask, int *rc)

Please make the function return a value (perhaps bool) instead.

> @@ -451,18 +455,28 @@ static void __init parse_psr_param(char *s)
>          if ( val_str )
>              *val_str++ = '\0';
>  
> -        parse_psr_bool(s, val_str, "cmt", PSR_CMT);
> -        parse_psr_bool(s, val_str, "cat", PSR_CAT);
> -        parse_psr_bool(s, val_str, "cdp", PSR_CDP);
> +        parse_psr_bool(s, val_str, "cmt", PSR_CMT, &rc);
> +        parse_psr_bool(s, val_str, "cat", PSR_CAT, &rc);
> +        parse_psr_bool(s, val_str, "cdp", PSR_CDP, &rc);
>  
>          if ( val_str && !strcmp(s, "rmid_max") )
> -            opt_rmid_max = simple_strtoul(val_str, NULL, 0);
> +        {
> +            opt_rmid_max = simple_strtoul(val_str, &q, 0);
> +            if ( *q )
> +                rc = -EINVAL;
> +        }
>  
>          if ( val_str && !strcmp(s, "cos_max") )
> -            opt_cos_max = simple_strtoul(val_str, NULL, 0);
> +        {
> +            opt_cos_max = simple_strtoul(val_str, &q, 0);
> +            if ( *q )
> +                rc = -EINVAL;
> +        }
>  
>          s = ss + 1;
>      } while ( ss );

So if val_str didn't match any of the five strings you won't indicate
an error here, which seems inconsistent.

Jan
Jürgen Groß Aug. 14, 2017, 2:25 p.m. UTC | #2 
On 14/08/17 15:35, Jan Beulich wrote:
>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote:
>> --- a/xen/arch/x86/psr.c
>> +++ b/xen/arch/x86/psr.c
>> @@ -420,7 +420,7 @@ static const struct feat_props l2_cat_props = {
>>  };
>>  
>>  static void __init parse_psr_bool(char *s, char *value, char *feature,
>> -                                  unsigned int mask)
>> +                                  unsigned int mask, int *rc)
> 
> Please make the function return a value (perhaps bool) instead.

Okay.

> 
>> @@ -451,18 +455,28 @@ static void __init parse_psr_param(char *s)
>>          if ( val_str )
>>              *val_str++ = '\0';
>>  
>> -        parse_psr_bool(s, val_str, "cmt", PSR_CMT);
>> -        parse_psr_bool(s, val_str, "cat", PSR_CAT);
>> -        parse_psr_bool(s, val_str, "cdp", PSR_CDP);
>> +        parse_psr_bool(s, val_str, "cmt", PSR_CMT, &rc);
>> +        parse_psr_bool(s, val_str, "cat", PSR_CAT, &rc);
>> +        parse_psr_bool(s, val_str, "cdp", PSR_CDP, &rc);
>>  
>>          if ( val_str && !strcmp(s, "rmid_max") )
>> -            opt_rmid_max = simple_strtoul(val_str, NULL, 0);
>> +        {
>> +            opt_rmid_max = simple_strtoul(val_str, &q, 0);
>> +            if ( *q )
>> +                rc = -EINVAL;
>> +        }
>>  
>>          if ( val_str && !strcmp(s, "cos_max") )
>> -            opt_cos_max = simple_strtoul(val_str, NULL, 0);
>> +        {
>> +            opt_cos_max = simple_strtoul(val_str, &q, 0);
>> +            if ( *q )
>> +                rc = -EINVAL;
>> +        }
>>  
>>          s = ss + 1;
>>      } while ( ss );
> 
> So if val_str didn't match any of the five strings you won't indicate
> an error here, which seems inconsistent.

Okay, I'll change it.


Juergen
diff mbox

Patch

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 9ce8f17a18..397963c667 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -420,7 +420,7 @@  static const struct feat_props l2_cat_props = {
 };
 
 static void __init parse_psr_bool(char *s, char *value, char *feature,
-                                  unsigned int mask)
+                                  unsigned int mask, int *rc)
 {
     if ( !strcmp(s, feature) )
     {
@@ -434,13 +434,17 @@  static void __init parse_psr_bool(char *s, char *value, char *feature,
                 opt_psr &= ~mask;
             else if ( val_int == 1 )
                 opt_psr |= mask;
+            else
+                *rc = -EINVAL;
         }
     }
 }
 
-static void __init parse_psr_param(char *s)
+static int __init parse_psr_param(char *s)
 {
     char *ss, *val_str;
+    const char *q;
+    int rc = 0;
 
     do {
         ss = strchr(s, ',');
@@ -451,18 +455,28 @@  static void __init parse_psr_param(char *s)
         if ( val_str )
             *val_str++ = '\0';
 
-        parse_psr_bool(s, val_str, "cmt", PSR_CMT);
-        parse_psr_bool(s, val_str, "cat", PSR_CAT);
-        parse_psr_bool(s, val_str, "cdp", PSR_CDP);
+        parse_psr_bool(s, val_str, "cmt", PSR_CMT, &rc);
+        parse_psr_bool(s, val_str, "cat", PSR_CAT, &rc);
+        parse_psr_bool(s, val_str, "cdp", PSR_CDP, &rc);
 
         if ( val_str && !strcmp(s, "rmid_max") )
-            opt_rmid_max = simple_strtoul(val_str, NULL, 0);
+        {
+            opt_rmid_max = simple_strtoul(val_str, &q, 0);
+            if ( *q )
+                rc = -EINVAL;
+        }
 
         if ( val_str && !strcmp(s, "cos_max") )
-            opt_cos_max = simple_strtoul(val_str, NULL, 0);
+        {
+            opt_cos_max = simple_strtoul(val_str, &q, 0);
+            if ( *q )
+                rc = -EINVAL;
+        }
 
         s = ss + 1;
     } while ( ss );
+
+    return rc;
 }
 custom_param("psr", parse_psr_param);