Re: [Xen-devel] [PATCH v3 09/52] xen/arch/x86/hvm/viridian.c: let custom parameter parsing routines return errno

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Juergen Gross [mailto:jgross@xxxxxxxx]
> Sent: 16 August 2017 13:52
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Juergen Gross <jgross@xxxxxxxx>; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; Andrew
> Cooper <Andrew.Cooper3@xxxxxxxxxx>
> Subject: [PATCH v3 09/52] xen/arch/x86/hvm/viridian.c: let custom
> parameter parsing routines return errno
> 
> Modify the custom parameter parsing routines in:
> 
> xen/arch/x86/hvm/viridian.c
> 
> to indicate whether the parameter value was parsed successfully.
> 
> Fix an error in the parsing function: up to now it would overwrite the
> stack in case more than 3 values are specified.
> 
> Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> V3:
> - dont modify option value in parsing function
> - fix error in parsing routine
> ---
>  xen/arch/x86/hvm/viridian.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index aa9b87c0ab..2edf9d0b23 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -1083,7 +1083,7 @@ static int viridian_load_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
>  HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_VCPU, viridian_save_vcpu_ctxt,
>                            viridian_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
> 
> -static void __init parse_viridian_version(char *arg)
> +static int __init parse_viridian_version(const char *arg)
>  {
>      const char *t;
>      unsigned int n[3];
> @@ -1093,17 +1093,24 @@ static void __init parse_viridian_version(char
> *arg)
>      n[1] = viridian_minor;
>      n[2] = viridian_build;
> 
> -    while ( (t = strsep(&arg, ",")) != NULL )
> -    {
> +    do {
>          const char *e;
> 
> -        if ( *t == '\0' )
> -            continue;
> +        t = strchr(arg, ',');
> +        if ( !t )
> +            t = strchr(arg, '\0');
> +
> +        if ( *arg && *arg != ',' && i < 3 )
> +        {
> +            n[i] = simple_strtoul(arg, &e, 0);
> +            if ( e != t )
> +                goto fail;
> +        }
> +
> +        i++;
> +        arg = t + 1;
> +    } while ( *t );

The loop is much neater when strsep() is used. Could you not just add a check 
for i < 3 at the beginning?

  Paul

> 
> -        n[i++] = simple_strtoul(t, &e, 0);
> -        if ( *e != '\0' )
> -            goto fail;
> -    }
>      if ( i != 3 )
>          goto fail;
> 
> @@ -1118,10 +1125,11 @@ static void __init parse_viridian_version(char
> *arg)
> 
>      printk("viridian-version = %#x,%#x,%#x\n",
>             viridian_major, viridian_minor, viridian_build);
> -    return;
> +    return 0;
> 
>   fail:
>      printk(XENLOG_WARNING "Invalid viridian-version, using default\n");
> +    return -EINVAL;
>  }
>  custom_param("viridian-version", parse_viridian_version);
> 
> --
> 2.12.3


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