[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 09/52] xen/arch/x86/hvm/viridian.c: let custom parameter parsing routines return errno
> -----Original Message----- > From: Juergen Gross [mailto:jgross@xxxxxxxx] > Sent: 21 August 2017 12:02 > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx > Cc: Jan Beulich <jbeulich@xxxxxxxx>; Andrew Cooper > <Andrew.Cooper3@xxxxxxxxxx> > Subject: Re: [PATCH v3 09/52] xen/arch/x86/hvm/viridian.c: let custom > parameter parsing routines return errno > > On 21/08/17 10:33, Paul Durrant wrote: > >> -----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? > > Of course I could. OTOH I don't think the resulting code would be > neater. I can't remove the check for i being 3 after the loop, so > I'd have to add some lines instead of testing i < 3 in the already > present if statement. > > In case you are targeting to continue using strsep(): this would > modify the option string. I want to avoid that as I need it unmodified > for being able to use it in the error message of patch 39. Ok, I don't feel that strongly. If you believe this is the neatest way... Acked-by: Paul Durrant <paul.durrant@xxxxxxxxxx> > > > Juergen _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |