Re: [PATCH 1/3] memtest: use kstrtouint instead of simple_strtoul

[Vladimir Murzin <vladimir.murzin@arm.com>]

On 22/06/15 11:43, Leon Romanovsky wrote:
> On Mon, Jun 22, 2015 at 11:39 AM, Vladimir Murzin
> <vladimir.murzin@arm.com> wrote:
>> On 20/06/15 07:55, Leon Romanovsky wrote:
>>> On Fri, Jun 19, 2015 at 5:58 PM, Vladimir Murzin
>>> <vladimir.murzin@arm.com> wrote:
>>>> Since simple_strtoul is obsolete and memtest_pattern is type of int, use
>>>> kstrtouint instead.
>>>>
>>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>>> ---
>>>>  mm/memtest.c |   14 +++++++++-----
>>>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/mm/memtest.c b/mm/memtest.c
>>>> index 1997d93..895a43c 100644
>>>> --- a/mm/memtest.c
>>>> +++ b/mm/memtest.c
>>>> @@ -88,14 +88,18 @@ static void __init do_one_pass(u64 pattern, phys_addr_t start, phys_addr_t end)
>>>>  }
>>>>
>>>>  /* default is disabled */
>>>> -static int memtest_pattern __initdata;
>>>> +static unsigned int memtest_pattern __initdata;
>>>>
>>>>  static int __init parse_memtest(char *arg)
>>>>  {
>>>> -       if (arg)
>>>> -               memtest_pattern = simple_strtoul(arg, NULL, 0);
>>>> -       else
>>>> -               memtest_pattern = ARRAY_SIZE(patterns);
>>>> +       if (arg) {
>>>> +               int err = kstrtouint(arg, 0, &memtest_pattern);
>>>> +
>>>> +               if (!err)
>>>> +                       return 0;
>>> kstrtouint returns 0 for success, in case of error you will fallback
>>> and execute following line. It is definetely change of behaviour.
>>
>> I'd be glad if you can elaborate more on use cases dependent on this
>> change? I can only imagine providing garbage to the memtest option with
>> only intention to shut it up... but it looks like the interface abuse
>> since "memtest=0" does exactly the same.
>>
>> Since memtest is debugging option and numeric parameter is optional I
>> thought it was not harmful to fallback to default in case something is
>> wrong with the parameter.
> I would like to suggest you, in case of error, print warning and set
> memtest_pattern to be zero and return back to if(arg)..else
> construction.

In case of kstrtouint() failure memtest_pattern is not altered, so I
won't need extra assignment here... and I'm not sure I get your point
with returning back to if(arg)...else

Anyway, I'd wait for other comments on what should be done here if anything.

Thanks
Vladimir

> 
>>
>> Thanks
>> Vladimir
>>
>>>> +       }
>>>> +
>>>> +       memtest_pattern = ARRAY_SIZE(patterns);
>>>>
>>>>         return 0;
>>>>  }
>>>> --
>>>> 1.7.9.5
>>>>
>>>> --
>>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>>> the body to majordomo@kvack.org.  For more info on Linux MM,
>>>> see: http://www.linux-mm.org/ .
>>>> Don't email: <a hrefmailto:"dont@kvack.org"> email@kvack.org </a>
>>>
>>>
>>>
>>
> 
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>