[PATCH] staging: speakup: remove simple_strtoul()

[Justin Skists]

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.

Also, I've noticed a couple of issues with your original submission.

1) You need to send patches in plain-text :)
2) You forgot to add devel at driverdev.osuosl.org (staging drivers subsystem) to the cc: list.

Hope that helps,
Justin.