[PATCH v2] staging: speakup: replace simple_strtoul by kstrtoul

[PATCH v2] staging: speakup: replace simple_strtoul by kstrtoul

[Gargi Sharma]

Making changes to add a check to the value returned by kstrtoul. Also,
updating temp since it is being used afterwards.

Signed-off-by: Gargi Sharma <gs051095@gmail.com>
---
 drivers/staging/speakup/kobjects.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index 884b2eb..e42be0f 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -154,7 +154,13 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 			continue;
 		}
 
-		err = kstrtoul(cp, 10, &index);
+		temp = cp;
+		err = kstrtoul(temp, 10, &index);
+		if (err < 0) {
+			retval = err;
+			break;
+		}
+
 		if (index > 255) {
 			rejected++;
 			cp = linefeed + 1;
@@ -785,7 +791,12 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 			continue;
 		}
 
-		err = kstrtoul(cp, 10, &index);
+		temp = cp;
+		err = kstrtoul(temp, 10, &index);
+		if (err < 0) {
+			retval = err;
+			break;
+		}
 
 		while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
 			temp++;
-- 
2.9.2

Re: [Outreachy kernel] [PATCH v2] staging: speakup: replace simple_strtoul by kstrtoul

[Julia Lawall]

On Tue, 13 Sep 2016, Gargi Sharma wrote:

> Making changes to add a check to the value returned by kstrtoul. Also,
> updating temp since it is being used afterwards.

This patch doesn't do what the subject says.  It looks like you picked up
from your previous patch, whereas you should have picked up from the state
before that patch, to do the whole thing at once.


> Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> ---
>  drivers/staging/speakup/kobjects.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
> index 884b2eb..e42be0f 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -154,7 +154,13 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
>  			continue;
>  		}
>
> -		err = kstrtoul(cp, 10, &index);
> +		temp = cp;

I don't think this is correct.  Look at the definition of simple_strtoul
and see what the second argument is supposed to end up being initialized
to.

I suspect that it is not possible, or at least not very convenient, to use
kstrtoul in these cases.

julia

> +		err = kstrtoul(temp, 10, &index);
> +		if (err < 0) {
> +			retval = err;
> +			break;
> +		}
> +
>  		if (index > 255) {
>  			rejected++;
>  			cp = linefeed + 1;
> @@ -785,7 +791,12 @@ static ssize_t message_store_helper(const char *buf, size_t count,
>  			continue;
>  		}
>
> -		err = kstrtoul(cp, 10, &index);
> +		temp = cp;
> +		err = kstrtoul(temp, 10, &index);
> +		if (err < 0) {
> +			retval = err;
> +			break;
> +		}
>
>  		while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
>  			temp++;
> --
> 2.9.2
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160913133212.7830-1-gs051095%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>