[PATCH] staging: speakup: Replace simple_strtoul by kstrtoul

[PATCH] staging: speakup: Replace simple_strtoul by kstrtoul

[Gargi Sharma]

simple_strtoul is obsolete and hence has been replaced by kstrtoul as
mentioned by checkpatch.pl

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

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index 528cbdc..884b2eb 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -126,6 +126,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 	int do_characters = !strcmp(attr->attr.name, "characters");
 	size_t desc_length = 0;
 	int i;
+	int err;
 
 	spin_lock_irqsave(&speakup_info.spinlock, flags);
 	while (cp < end) {
@@ -153,7 +154,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		err = kstrtoul(cp, 10, &index);
 		if (index > 255) {
 			rejected++;
 			cp = linefeed + 1;
@@ -757,6 +758,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 	enum msg_index_t firstmessage = group->start;
 	enum msg_index_t lastmessage = group->end;
 	enum msg_index_t curmessage;
+	int err;
 
 	while (cp < end) {
 
@@ -783,7 +785,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		err = kstrtoul(cp, 10, &index);
 
 		while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
 			temp++;
-- 
2.9.2

Re: [PATCH] staging: speakup: Replace simple_strtoul by kstrtoul

[Gargi Sharma]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On Tue, Sep 13, 2016 at 2:26 AM, Samuel Thibault <
samuel.thibault@ens-lyon.org> wrote:
>
> Hello,
>
> Gargi Sharma, on Tue 13 Sep 2016 02:20:44 +0530, wrote:
> > simple_strtoul is obsolete and hence has been replaced by kstrtoul as
> > mentioned by checkpatch.pl
>
> Please note the details.
>
> > diff --git a/drivers/staging/speakup/kobjects.c
b/drivers/staging/speakup/kobjects.c
> > index 528cbdc..884b2eb 100644
> > --- a/drivers/staging/speakup/kobjects.c
> > +++ b/drivers/staging/speakup/kobjects.c
> > @@ -126,6 +126,7 @@ static ssize_t chars_chartab_store(struct kobject
*kobj,
> >       int do_characters = !strcmp(attr->attr.name, "characters");
> >       size_t desc_length = 0;
> >       int i;
> > +     int err;
> >
> >       spin_lock_irqsave(&speakup_info.spinlock, flags);
> >       while (cp < end) {
> > @@ -153,7 +154,7 @@ static ssize_t chars_chartab_store(struct kobject
*kobj,
> >                       continue;
> >               }
> >
> > -             index = simple_strtoul(cp, &temp, 10);
> > +             err = kstrtoul(cp, 10, &index);
> >               if (index > 255) {
> >                       rejected++;
> >                       cp = linefeed + 1;
>
> Here it would be good to include err in the if test, so as to properly
reject
> non-numeric values.

temp is used here as well and again is not updated. So, I think kstrtoul
can't be used here as well,

thanks,
gargi

[-- Attachment #2: Type: text/html, Size: 1979 bytes --]

[PATCH] staging: speakup: Replace simple_strtoul by kstrtoul

[Shekhar Bhandakkar]

The simple_strtoul function is obsolete.
This patch replaces it by kstrtoul.

Signed-off-by: Shekhar Bhandakkar <cs14btech11006@iith.ac.in>
---
 drivers/staging/speakup/kobjects.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index ca85476..8b9ddb9 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -127,6 +127,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 	int do_characters = !strcmp(attr->attr.name, "characters");
 	size_t desc_length = 0;
 	int i;
+	int err;
 
 	spin_lock_irqsave(&speakup_info.spinlock, flags);
 	while (cp < end) {
@@ -153,7 +154,9 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		err = kstrtoul(cp, 10, &index);
+		if (err)
+			return err;
 		if (index > 255) {
 			rejected++;
 			cp = linefeed + 1;
-- 
2.7.4

Re: [PATCH] staging: speakup: Replace simple_strtoul by kstrtoul

[Samuel Thibault]

Shekhar Bhandakkar, on jeu. 27 avril 2017 22:10:32 +0530, wrote:
> The simple_strtoul function is obsolete.
> This patch replaces it by kstrtoul.

> @@ -153,7 +154,9 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
>  			continue;
>  		}
>  
> -		index = simple_strtoul(cp, &temp, 10);
> +		err = kstrtoul(cp, 10, &index);

Again, that doesn't update the temp pointer, which is used below.
So really NACK from me.

Samuel

[PATCH] staging: speakup: Replace simple_strtoul by kstrtoul

[Shekhar Bhandakkar]

The simple_strtoul function is obsolete.
This patch replaces it with kstrtoul.

Signed-off-by: Shekhar Bhandakkar <cs14btech11006@iith.ac.in>
---
 drivers/staging/speakup/kobjects.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index ca85476..8a1ebe6 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -127,6 +127,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 	int do_characters = !strcmp(attr->attr.name, "characters");
 	size_t desc_length = 0;
 	int i;
+	int err;
 
 	spin_lock_irqsave(&speakup_info.spinlock, flags);
 	while (cp < end) {
@@ -153,7 +154,10 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		temp = cp;
+		err = kstrtoul(temp, 10, &index);
+		if (err)
+			return err;
 		if (index > 255) {
 			rejected++;
 			cp = linefeed + 1;
-- 
2.7.4

Re: [PATCH] staging: speakup: Replace simple_strtoul by kstrtoul

[Julia Lawall]

On Fri, 28 Apr 2017, Shekhar Bhandakkar wrote:

> The simple_strtoul function is obsolete.
> This patch replaces it with kstrtoul.
>
> Signed-off-by: Shekhar Bhandakkar <cs14btech11006@iith.ac.in>
> ---
>  drivers/staging/speakup/kobjects.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
> index ca85476..8a1ebe6 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -127,6 +127,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
>  	int do_characters = !strcmp(attr->attr.name, "characters");
>  	size_t desc_length = 0;
>  	int i;
> +	int err;
>
>  	spin_lock_irqsave(&speakup_info.spinlock, flags);
>  	while (cp < end) {
> @@ -153,7 +154,10 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
>  			continue;
>  		}
>
> -		index = simple_strtoul(cp, &temp, 10);
> +		temp = cp;
> +		err = kstrtoul(temp, 10, &index);

I haven't looked at how temp is used, but I doubt that this is the
correct fix, because it doesn't make the distinction between cp and temp
meaningful.  In my experience, all of the remaining uses of
simple_strtoul cannot be converted to kstrtoul because they use the second
argument of simple_strtoul in some important way.  The checkpatch warning
is more intended to suggest that people avoid adding new uses of
simple_strtoul rather than an indication that it is possible to convert
all the old ones.

In any case, when you make a change in a patch, it should becomes [PATCH
v2] and under the --- you should explain what is new in the v2.  In a case
like this where the change is complex, above the --- you should also
explain why all aspects of your change are correct, in particular about
the corrcetness of the use of temp.

julia

> +		if (err)
> +			return err;
>  		if (index > 255) {
>  			rejected++;
>  			cp = linefeed + 1;
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] staging: speakup: Replace simple_strtoul by kstrtoul

[Dan Carpenter]

I happened to search my gmail spam box for patches and found this.

On Fri, Apr 28, 2017 at 6:50 AM, Shekhar Bhandakkar
<cs14btech11006@iith.ac.in> wrote:
> The simple_strtoul function is obsolete.
> This patch replaces it with kstrtoul.
>
> Signed-off-by: Shekhar Bhandakkar <cs14btech11006@iith.ac.in>
> ---
>  drivers/staging/speakup/kobjects.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
> index ca85476..8a1ebe6 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -127,6 +127,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
>         int do_characters = !strcmp(attr->attr.name, "characters");
>         size_t desc_length = 0;
>         int i;
> +       int err;
>
>         spin_lock_irqsave(&speakup_info.spinlock, flags);
>         while (cp < end) {
> @@ -153,7 +154,10 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
>                         continue;
>                 }
>
> -               index = simple_strtoul(cp, &temp, 10);
> +               temp = cp;
> +               err = kstrtoul(temp, 10, &index);
> +               if (err)
> +                       return err;

This is buggy.  "temp" is supposed to point to the end of the number.

regards,
dan carpenter