On Thu, Apr 11, 2013 at 10:46 PM, HATAYAMA Daisuke
<d.hatay...@jp.fujitsu.com> wrote:
> (2013/04/11 4:39), Yinghai Lu wrote:
> <cut>
>> Index: linux-2.6/kernel/kexec.c
>> ===================================================================
>> --- linux-2.6.orig/kernel/kexec.c
>> +++ linux-2.6/kernel/kexec.c
>> @@ -1368,35 +1368,108 @@ static int __init parse_crashkernel_simp
>> return 0;
>> }
>>
>> +#define SUFFIX_HIGH 0
>> +#define SUFFIX_LOW 1
>> +#define SUFFIX_NULL 2
>> +static __initdata char *suffix_tbl[] = {
>> + [SUFFIX_HIGH] = ",high",
>> + [SUFFIX_LOW] = ",low",
>> + [SUFFIX_NULL] = NULL,
>> +};
>> +
>> /*
>> - * That function is the entry point for command line parsing and should be
>> - * called from the arch-specific code.
>> + * That function parses "suffix" crashkernel command lines like
>> + *
>> + * crashkernel=size,[high|low]
>> + *
>> + * It returns 0 on success and -EINVAL on failure.
>> */
>> +static int __init parse_crashkernel_suffix(char *cmdline,
>> + unsigned long long *crash_size,
>> + unsigned long long *crash_base,
>> + const char *suffix)
>> +{
>> + char *cur = cmdline;
>> +
>> + *crash_size = memparse(cmdline, &cur);
>> + if (cmdline == cur) {
>> + pr_warn("crashkernel: memory value expected\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* check with suffix */
>> + if (!strncmp(cur, suffix, strlen(suffix)))
>> + return 0;
>
> What kind of strings do you intend to be passed here? The syntax that
> matches this check would be
>
> [0-9]+[kmgKMG]?(,low|,high){.*(,low|,high)}*
should be
[0-9]+[kmgKMG](,low|,high)
will tight the checking.
>
> though memparse() part might be less precise. For example, the following
> one passes the check since it ends with ",low" and ",high" follows
> "12345K" to be parsed by memparse(). Is this within your intension for
> ease of implementation?
>
> crashkernel=12345K,highfoobar,highabcd,low
it will be rejected.
crashkernel=12345K,highfoobar,highabcd,high
should be rejected too.
>
>> +
>> + pr_warn("crashkernel: unrecognized char\n");
>> + return -EINVAL;
>> +}
>
> I like functions checking errornous cases in the middle and ends with
> successful case. How about:
>
> /* check with suffix */
> if (strncmp(cur, suffix, strlen(suffix)) {
> pr_warn("crashkernel: unrecognized char\n");
> return -EINVAL;
> }
>
> return 0;
ok.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
