[PATCH] staging: speakup: replace simple_strtoul with kstrtoul

[PATCH] staging: speakup: replace simple_strtoul with kstrtoul

[samuel kihahu]

Replacing obsolete simple_strtoul with kstrtoul.

Signed-off-by: samuel kihahu <skihahu@gmail.com>
---
 drivers/staging/speakup/varhandlers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/varhandlers.c b/drivers/staging/speakup/varhandlers.c
index d758284..b526c8e 100644
--- a/drivers/staging/speakup/varhandlers.c
+++ b/drivers/staging/speakup/varhandlers.c
@@ -323,7 +323,7 @@ char *spk_s2uchar(char *start, char *dest)
 {
 	int val = 0;
 
-	val = simple_strtoul(skip_spaces(start), &start, 10);
+	val = kstrtoul(skip_spaces(start), &start, 10);
 	if (*start == ',')
 		start++;
 	*dest = (u_char)val;
-- 
1.8.3.1

Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul

[Dan Carpenter]

On Wed, Dec 17, 2014 at 02:56:02PM +0300, samuel kihahu wrote:
> Replacing obsolete simple_strtoul with kstrtoul.
> 

Nope.  That's wrong.  Learn how the functions are different beyond just
the name.

regards,
dan carpenter

Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul

[samuel kihahu]

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

On Wed, Dec 17, 2014 at 03:11:19PM +0300, Dan Carpenter wrote:
> On Wed, Dec 17, 2014 at 02:56:02PM +0300, samuel kihahu wrote:
> > Replacing obsolete simple_strtoul with kstrtoul.
> > 
> 
> Nope.  That's wrong.  Learn how the functions are different beyond just
> the name.
Noted, have made corrections to fit the kstrtoul and handle the return
value.

[-- Attachment #2: 0001-staging-speakup-replace-simple_strtoul-with-kstrtoul.patch --]
[-- Type: text/plain, Size: 992 bytes --]

>From 71d2f7123e697b95371585b4af9a0b35262307ea Mon Sep 17 00:00:00 2001
From: samuel kihahu <skihahu@gmail.com>
Date: Wed, 17 Dec 2014 16:31:05 +0300
Subject: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul

Replacing obsolete simple_strtoul with kstrtoul and checking the
return status.

Signed-off-by: samuel kihahu <skihahu@gmail.com>
---
 drivers/staging/speakup/varhandlers.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/varhandlers.c b/drivers/staging/speakup/varhandlers.c
index d758284..8ac9ad7 100644
--- a/drivers/staging/speakup/varhandlers.c
+++ b/drivers/staging/speakup/varhandlers.c
@@ -321,9 +321,10 @@ char *spk_strlwr(char *s)
 
 char *spk_s2uchar(char *start, char *dest)
 {
-	int val = 0;
+	unsigned long val;
 
-	val = simple_strtoul(skip_spaces(start), &start, 10);
+	if (kstrtoul(skip_spaces(start), &start, val))
+		return -EINVAL;
 	if (*start == ',')
 		start++;
 	*dest = (u_char)val;
-- 
1.8.3.1

Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul

[Dan Carpenter]

On Wed, Dec 17, 2014 at 04:43:54PM +0300, samuel kihahu wrote:
> On Wed, Dec 17, 2014 at 03:11:19PM +0300, Dan Carpenter wrote:
> > On Wed, Dec 17, 2014 at 02:56:02PM +0300, samuel kihahu wrote:
> > > Replacing obsolete simple_strtoul with kstrtoul.
> > > 
> > 
> > Nope.  That's wrong.  Learn how the functions are different beyond just
> > the name.
> Noted, have made corrections to fit the kstrtoul and handle the return
> value.

You have to compile test these things.  Really kernel programming is not
a good way to learn how to program.  :(

regards,
dan carpenter

Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul

[samuel kihahu]

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

On Wed, Dec 17, 2014 at 05:03:22PM +0300, Dan Carpenter wrote:
> On Wed, Dec 17, 2014 at 04:43:54PM +0300, samuel kihahu wrote:
> > On Wed, Dec 17, 2014 at 03:11:19PM +0300, Dan Carpenter wrote:
> > > On Wed, Dec 17, 2014 at 02:56:02PM +0300, samuel kihahu wrote:
> > > > Replacing obsolete simple_strtoul with kstrtoul.
> > > > 
> > > 
> > > Nope.  That's wrong.  Learn how the functions are different beyond just
> > > the name.
> > Noted, have made corrections to fit the kstrtoul and handle the return
> > value.
> 
> You have to compile test these things.  Really kernel programming is not
> a good way to learn how to program.  :(
> 
> regards,
> dan carpenter
> 

Appreciate the feedback, have modified the patch, tested and confirmed
it builds.

[-- Attachment #2: 0001-staging-speakup-replace-simple_strtoul-with-kstrtoul.patch --]
[-- Type: text/plain, Size: 1050 bytes --]

>From 3d39fd533b4a75e567f76f07f5bc978adddb0721 Mon Sep 17 00:00:00 2001
From: samuel kihahu <skihahu@gmail.com>
Date: Thu, 18 Dec 2014 17:42:37 +0300
Subject: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul

Replacing obsolete simple_strtoul with kstrtoul, checking and
returning the correct error code.

Signed-off-by: samuel kihahu <skihahu@gmail.com>
---
 drivers/staging/speakup/varhandlers.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/varhandlers.c b/drivers/staging/speakup/varhandlers.c
index d758284..716ccc1 100644
--- a/drivers/staging/speakup/varhandlers.c
+++ b/drivers/staging/speakup/varhandlers.c
@@ -321,9 +321,13 @@ char *spk_strlwr(char *s)
 
 char *spk_s2uchar(char *start, char *dest)
 {
-	int val = 0;
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(skip_spaces(start), 10, &val);
+	if (IS_ERR(&ret))
+		return ERR_CAST(&ret);
 
-	val = simple_strtoul(skip_spaces(start), &start, 10);
 	if (*start == ',')
 		start++;
 	*dest = (u_char)val;
-- 
1.8.3.1

Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul

[Dan Carpenter]

On Thu, Dec 18, 2014 at 05:48:10PM +0300, samuel kihahu wrote:
> On Wed, Dec 17, 2014 at 05:03:22PM +0300, Dan Carpenter wrote:
> > On Wed, Dec 17, 2014 at 04:43:54PM +0300, samuel kihahu wrote:
> > > On Wed, Dec 17, 2014 at 03:11:19PM +0300, Dan Carpenter wrote:
> > > > On Wed, Dec 17, 2014 at 02:56:02PM +0300, samuel kihahu wrote:
> > > > > Replacing obsolete simple_strtoul with kstrtoul.
> > > > > 
> > > > 
> > > > Nope.  That's wrong.  Learn how the functions are different beyond just
> > > > the name.
> > > Noted, have made corrections to fit the kstrtoul and handle the return
> > > value.
> > 
> > You have to compile test these things.  Really kernel programming is not
> > a good way to learn how to program.  :(
> > 
> > regards,
> > dan carpenter
> > 
> 
> Appreciate the feedback, have modified the patch, tested and confirmed
> it builds.

No.  It's still really wrong.  One of the key differences between
kstrtoul() and simple_strtoul() is that simple_strtoul() gives you a
pointer to the end of the string.

It's actually best to use simple_strtoul() here.  Checkpatch.pl is
wrong.

As well the new error handling doesn't work at all.

regards,
dan carpenter

[PATCH] staging: speakup: Replace simple_strtoul with kstrtoul

[Amitoj Kaur Chawla]

Replace obsolete simple_strtoul with kstrtoul.
The Coccinelle semantic patch used to make this change is as follows:

// <smpl>
@@
unsigned long l;
expression e,f,g;
@@
- l = simple_strtoul(e, &f, g)
+ kstrtoul(e, g, &l)
// </smpl>

Additionally, since kstrtoul returns 0 on success and an error code on
failure introduced `err` variable to store and return the error code.
This change was made by hand.

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
 drivers/staging/speakup/kobjects.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index fdfeb42..f626084 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -125,7 +125,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 	int reset = 0;
 	int do_characters = !strcmp(attr->attr.name, "characters");
 	size_t desc_length = 0;
-	int i;
+	int i, err;
 
 	spin_lock_irqsave(&speakup_info.spinlock, flags);
 	while (cp < end) {
@@ -153,7 +153,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;
@@ -757,6 +759,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 +786,9 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		err = kstrtoul(cp, 10, &index);
+		if (err)
+			return err;
 
 		while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
 			temp++;
-- 
1.9.1

[PATCH] Staging: speakup: Replace simple_strtoul with kstrtoul

[Mihaela Muraru]

This patch fixes up a checkpatch.pl warning :
WARNING: simple_strtoul is obsolete, use kstrtoul instead.

Signed-off-by: Mihaela Muraru <mihaela.muraru21@gmail.com>
---
 drivers/staging/speakup/kobjects.c | 5 ++++-
 drivers/staging/speakup/main.c     | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index e744aa9..6466a09 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -757,6 +757,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 	int used = 0;
 	int rejected = 0;
 	int reset = 0;
+	int ret;
 	enum msg_index_t firstmessage = group->start;
 	enum msg_index_t lastmessage = group->end;
 	enum msg_index_t curmessage;
@@ -786,7 +787,9 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		ret = kstrtoul(cp, 10, &index);
+		if (ret < 0)
+			return ret;
 
 		while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
 			temp++;
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 97ca4ec..6064fef 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1906,6 +1906,7 @@ static int handle_goto(struct vc_data *vc, u_char type, u_char ch, u_short key)
 {
 	static u_char goto_buf[8];
 	static int num;
+	int ret;
 	int maxlen;
 	char *cp;
 
@@ -1944,7 +1945,9 @@ oops:
 		return 1;
 	}
 
-	goto_pos = simple_strtoul(goto_buf, &cp, 10);
+	ret = kstrtoul(goto_buf, 10, &goto_pos);
+	if (ret < 0)
+		return ret;
 
 	if (*cp == 'x') {
 		if (*goto_buf < '0')
-- 
2.7.4

Re: [PATCH] Staging: speakup: Replace simple_strtoul with kstrtoul

[Greg Kroah-Hartman]

On Tue, Oct 11, 2016 at 09:18:51AM +0300, Mihaela Muraru wrote:
> This patch fixes up a checkpatch.pl warning :
> WARNING: simple_strtoul is obsolete, use kstrtoul instead.

Please look at the mailing list archives for why I'm pretty sure this
does not work.  Have you verified that the code still works the same as
before by looking at how the data is handled?

thanks,

greg k-h

Re: [PATCH] Staging: speakup: Replace simple_strtoul with kstrtoul

[Muraru Mihaela]

On Tue, Oct 11, 2016 at 08:47:06AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 11, 2016 at 09:18:51AM +0300, Mihaela Muraru wrote:
> > This patch fixes up a checkpatch.pl warning :
> > WARNING: simple_strtoul is obsolete, use kstrtoul instead.
> 
> Please look at the mailing list archives for why I'm pretty sure this
> does not work.  Have you verified that the code still works the same as
> before by looking at how the data is handled?

Thank your for reply.

I am going to check it.

mihaela





> thanks,

> greg k-h

Re: [PATCH] Staging: speakup: Replace simple_strtoul with kstrtoul

[Muraru Mihaela]

On Tue, Oct 11, 2016 at 08:47:06AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 11, 2016 at 09:18:51AM +0300, Mihaela Muraru wrote:
> > This patch fixes up a checkpatch.pl warning :
> > WARNING: simple_strtoul is obsolete, use kstrtoul instead.
> 
> Please look at the mailing list archives for why I'm pretty sure this
> does not work.  Have you verified that the code still works the same as
> before by looking at how the data is handled?

After I read the mail list and after I did some researches indeed I
cannot change replace simple_strtoul with kstrtoul.

Thank you for telling me where to search.

mihaela

> thanks,
> 
> greg k-h

[PATCH] staging: speakup: Replace simple_strtoul with kstrtoul

[Nishka Dasgupta]

Replace simple_strtoul with kstrtoul. Issue found with checkpatch.

Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
---
 drivers/staging/speakup/kobjects.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index bd7cfab..836e1b7 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -756,6 +756,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 	ssize_t retval = count;
 	size_t desc_length = 0;
 	unsigned long index = 0;
+	unsigned long val = 0;
 	int received = 0;
 	int used = 0;
 	int rejected = 0;
@@ -788,7 +789,7 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		index = kstrtoul(cp, 10, &val);
 
 		while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
 			temp++;
-- 
1.9.1

[PATCH] staging: speakup: Replace simple_strtoul with kstrtoul

[Nishka Dasgupta]

Replace simple_strtoul with kstrtoul. Issue found with checkpatch.

Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
---
 drivers/staging/speakup/kobjects.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index f1f9022..bd7cfab 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -120,6 +120,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 	ssize_t retval = count;
 	unsigned long flags;
 	unsigned long index = 0;
+	unsigned long val = 0;
 	int charclass = 0;
 	int received = 0;
 	int used = 0;
@@ -154,7 +155,7 @@ static ssize_t chars_chartab_store(struct kobject *kobj,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		index = kstrtoul(cp, 10, &val);
 		if (index > 255) {
 			rejected++;
 			cp = linefeed + 1;
-- 
1.9.1

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

[Samuel Thibault]

Nishka Dasgupta, on ven. 09 mars 2018 00:51:16 +0530, wrote:
> Replace simple_strtoul with kstrtoul. Issue found with checkpatch.

kstrtoul is not the same as simple_strtoul, we need temp to be updated.

Samuel

[PATCH] staging: speakup: Replace simple_strtoul with kstrtoul

[Nishka Dasgupta]

Replace simple_strtoul with kstrtoul. Issue found with checkpatch.

Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
---
 drivers/staging/speakup/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index af30b70..9db7160 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1973,7 +1973,7 @@ static int handle_goto(struct vc_data *vc, u_char type, u_char ch, u_short key)
 		return 1;
 	}
 
-	goto_pos = simple_strtoul(goto_buf, &cp, 10);
+	kstrtoul(goto_buf, 10, &goto_pos);
 
 	if (*cp == 'x') {
 		if (*goto_buf < '0')
-- 
1.9.1

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

[Samuel Thibault]

Nishka Dasgupta, on ven. 09 mars 2018 01:13:46 +0530, wrote:
> Replace simple_strtoul with kstrtoul. Issue found with checkpatch.

kstrtoul is not the same as simple_strtoul. Here we need to have cp
updated.

Samuel

[PATCH] staging: speakup: Replace simple_strtoul with kstrtoul

[Nishka Dasgupta]

Replace simple_strtoul with kstrtoul. Issue found with checkpatch.

Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
---
 drivers/staging/speakup/varhandlers.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/varhandlers.c b/drivers/staging/speakup/varhandlers.c
index 3214055..e27bd5c 100644
--- a/drivers/staging/speakup/varhandlers.c
+++ b/drivers/staging/speakup/varhandlers.c
@@ -327,8 +327,9 @@ char *spk_strlwr(char *s)
 char *spk_s2uchar(char *start, char *dest)
 {
 	int val;
+	unsigned long v = 0;
 
-	val = simple_strtoul(skip_spaces(start), &start, 10);
+	val = kstrtoul(skip_spaces(start), 10, &v);
 	if (*start == ',')
 		start++;
 	*dest = (u_char)val;
-- 
1.9.1

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

[Samuel Thibault]

Nishka Dasgupta, on ven. 09 mars 2018 01:19:12 +0530, wrote:
> Replace simple_strtoul with kstrtoul. Issue found with checkpatch.

Again, they are not the same, we need start to be updated.

[PATCH] staging: speakup: Replace simple_strtoul with kstrtoul

[Bharath Vedartham]

simple_strtoul is obsolete. Change it to kstrtoul. Kernel is building
and booting successfully.

Signed-off-by: Bharath Vedartham <linux.bhar@gmail.com>
---
 drivers/staging/speakup/kobjects.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/kobjects.c
index 2e36d87..ad5a2b2 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -787,7 +787,9 @@ static ssize_t message_store_helper(const char *buf, size_t count,
 			continue;
 		}
 
-		index = simple_strtoul(cp, &temp, 10);
+		retval = kstrtoul(cp, 0, &index);
+		if (retval)
+			return retval;
 
 		while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
 			temp++;
-- 
2.7.4

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

[Samuel Thibault]

Bharath Vedartham, le dim. 24 févr. 2019 01:01:21 +0530, a ecrit:
> simple_strtoul is obsolete. Change it to kstrtoul. Kernel is building
> and booting successfully.

Please recheck your patch, temp is used after the simple_strtoul call.

Samuel

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

[Bharath Vedartham]

On Sat, Feb 23, 2019 at 08:37:50PM +0100, Samuel Thibault wrote:
> Bharath Vedartham, le dim. 24 févr. 2019 01:01:21 +0530, a ecrit:
> > simple_strtoul is obsolete. Change it to kstrtoul. Kernel is building
> > and booting successfully.
> 
> Please recheck your patch, temp is used after the simple_strtoul call.
> 
> Samuel

Sorry about that. After some thought, I feel this is a special case in
replacing simple_strtoul to kstrtoul. simple_strtoul can give back a
pointer to the end of the parsed string, whereas kstrtoul does not
give any thing of that kind. In our case, temp is the end of the
parsed cp string. 

To replace simple_strtoul to kstrtoul, we need to know the length of the
unsigned long int returned and shift the cp pointer by that length to
get temp. This might involve a bit of code refactoring.

Bharath