[PATCH] staging: speakup: remove simple_strtoul()

[Gabriel Fedel]

Hi Justin,

Em 21-05-2018 06:23, Justin Skists escreveu:
> Gabriel,
>
>> On 20 May 2018 at 21:06 Gabriel Fedel <fedel at fedel.net.br> wrote:
>>>>> Replace simple_strtoul() with kstrtoul(), because simple_strtoul() is
>>>>> obsolete
>>>>>
>>>>> Signed-off-by: Gabriel Fedel <fedel at fedel.net.br>
>>>>> ---
>>>>>  drivers/staging/speakup/kobjects.c | 7 +++++--
>>>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/speakup/kobjects.c
>>>>> b/drivers/staging/speakup/kobjects.c
>>>>> index f1f9022..ddc5ac3 100644
>>>>> --- a/drivers/staging/speakup/kobjects.c
>>>>> +++ b/drivers/staging/speakup/kobjects.c
>>>>> @@ -154,7 +154,9 @@ static ssize_t chars_chartab_store(struct kobject
>>>>> *kobj,
>>>>>  			continue;
>>>>>  		}
>>>>>
>>>>> -		index = simple_strtoul(cp, &temp, 10);
>>>>> +		if  kstrtoul((char *)cp, 10, &index) != 0
>>>>> +			pr_warn("overflow or parsing error has occurred");
>>>>> +
>>>>>  		if (index > 255) {
>>>>>  			rejected++;
>>>>>  			cp = linefeed + 1;
>>>>> @@ -787,7 +789,8 @@ static ssize_t message_store_helper(const char
>>>>> *buf,
>>>>> size_t count,
>>>>>  			continue;
>>>>>  		}
>>>>>
>>>>> -		index = simple_strtoul(cp, &temp, 10);
>>>>> +		if kstrtoul((char *)cp, 10, &index) != 0
>>>>> +			pr_warn("overflow or parsing error has occurred");
>>>>>
>>>>>  		while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
>>>>>  			temp++;
>>>>
>
>> Just to check, so is there no problem with temp on this change, right?
>
> I've had coffee, now. I'm afraid that there is a problem with temp in your change:
>
> simple_strtoul() would set temp to be the end of the parsed input.
>
> The function (chars_chartab_store() and message_store_helper()) is then using
> temp to skip some whitespace after the strtoul parsing.
>
> desc_length is calculated from the resulting string between temp and the linefeed.
>
> However, with the patch, temp hasn't been set (still NULL), so the desc_length
> calculations will be wrong.
You are right, I'm using kstrtoul considering the cp has only numbers.

>
> Also, I've noticed a couple of issues with your original submission.
>
> 1) You need to send patches in plain-text :)
Sorry! But I set up my email client for this... My email was not in text 
mode?

> 2) You forgot to add devel at driverdev.osuosl.org (staging drivers subsystem) to the cc: list.
Ok! I sent for the emails in MAINTAINERS file. I will add this email on 
next patch version.
>
> Hope that helps,
> Justin.
Thank you again,

Gabriel
>