Messages in this thread | | | Date | Tue, 9 Apr 2013 13:05:54 -0700 | Subject | Re: [PATCH v3 3/4] x86, kdump: Change crashkernel_high/low= to crashkernel=;high/low | From | Yinghai Lu <> |
| |
On Tue, Apr 9, 2013 at 6:45 AM, Vivek Goyal <vgoyal@redhat.com> wrote: > On Thu, Apr 04, 2013 at 03:17:01PM -0700, Yinghai Lu wrote: > > [..] >> @@ -1360,37 +1369,80 @@ static int __init parse_crashkernel_simp >> >> if (*cur == '@') >> *crash_base = memparse(cur+1, &cur); >> - else if (*cur != ' ' && *cur != '\0') { >> - pr_warning("crashkernel: unrecognized char\n"); >> - return -EINVAL; >> + else { >> + int i; >> + >> + /* check with known suffix */ >> + for (i = 0; suffix_tbl[i]; i++) >> + if (!strncmp(cur, suffix_tbl[i], strlen(suffix_tbl[i]))) >> + return 0; >> + > > So crashkernel=X@Y;high is a valid syntax? Looks like we will reserve > X amount of RAM at base Y and ignore "high" or "low".
yes, we should reject them.
> > [..] ... > > Why don't we structure it little differently. Now we seem to have 3 > categories of crashkernel= parameters. > > - crashkernel_simple (crashkernel=X or crashkernel=X@Y) > - crashkernel_mem (crashkernel=range:size,.....) > - crashkerenl_high_low_suffix (crashkernel=X;high or crashkernel=Y;low) > > if (suffix) { > parse_crashkernel_high_low_suffix() > } else { > if (first_colon.....) > parse_crashkernel_mem() > else > parse_crashkernel_simple(); > } > > And now you should not require "simple_only" function parameter and you > can also do strict syntax checking for each type of crashkernel= > parameter.
yes, that will the code more readable.
Just send -v4 of this patch that will not reuse parse_crashkernel_simple.
Thanks
Yinghai
| |