All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
@ 2018-03-08 19:43 Nishka Dasgupta
  2018-03-08 19:45 ` Samuel Thibault
  2018-03-08 20:51 ` [Outreachy kernel] " Julia Lawall
  0 siblings, 2 replies; 22+ messages in thread
From: Nishka Dasgupta @ 2018-03-08 19:43 UTC (permalink / raw)
  To: gregkh, w.d.hubbs, chris, kirk, samuel.thibault, outreachy-kernel
  Cc: 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



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
  2018-03-08 19:43 [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul Nishka Dasgupta
@ 2018-03-08 19:45 ` Samuel Thibault
  2018-03-08 20:51 ` [Outreachy kernel] " Julia Lawall
  1 sibling, 0 replies; 22+ messages in thread
From: Samuel Thibault @ 2018-03-08 19:45 UTC (permalink / raw)
  To: Nishka Dasgupta; +Cc: gregkh, w.d.hubbs, chris, kirk, outreachy-kernel

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Outreachy kernel] [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
  2018-03-08 19:43 [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul Nishka Dasgupta
  2018-03-08 19:45 ` Samuel Thibault
@ 2018-03-08 20:51 ` Julia Lawall
  1 sibling, 0 replies; 22+ messages in thread
From: Julia Lawall @ 2018-03-08 20:51 UTC (permalink / raw)
  To: Nishka Dasgupta
  Cc: gregkh, w.d.hubbs, chris, kirk, samuel.thibault, outreachy-kernel



On Fri, 9 Mar 2018, Nishka Dasgupta wrote:

> 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);

I haven't looked at this case in detail, but typically the cases that
still use simple_strtoul really need the simple_strtoul functionality.

julia

>
>  	if (*cp == 'x') {
>  		if (*goto_buf < '0')
> --
> 1.9.1
>
> --
> 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/1520538226-15590-1-git-send-email-nishka.dasgupta_ug18%40ashoka.edu.in.
> For more options, visit https://groups.google.com/d/optout.
>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
  2019-02-23 19:37 ` Samuel Thibault
@ 2019-02-23 20:02   ` Bharath Vedartham
  0 siblings, 0 replies; 22+ messages in thread
From: Bharath Vedartham @ 2019-02-23 20:02 UTC (permalink / raw)
  To: Samuel Thibault, w.d.hubbs, chris, kirk, gregkh, speakup, devel,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
  2019-02-23 19:31 Bharath Vedartham
@ 2019-02-23 19:37 ` Samuel Thibault
  2019-02-23 20:02   ` Bharath Vedartham
  0 siblings, 1 reply; 22+ messages in thread
From: Samuel Thibault @ 2019-02-23 19:37 UTC (permalink / raw)
  To: Bharath Vedartham
  Cc: w.d.hubbs, chris, kirk, gregkh, speakup, devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
@ 2019-02-23 19:31 Bharath Vedartham
  2019-02-23 19:37 ` Samuel Thibault
  0 siblings, 1 reply; 22+ messages in thread
From: Bharath Vedartham @ 2019-02-23 19:31 UTC (permalink / raw)
  To: w.d.hubbs
  Cc: chris, kirk, samuel.thibault, gregkh, speakup, devel, linux-kernel

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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
  2018-03-08 19:49 Nishka Dasgupta
@ 2018-03-08 19:53 ` Samuel Thibault
  0 siblings, 0 replies; 22+ messages in thread
From: Samuel Thibault @ 2018-03-08 19:53 UTC (permalink / raw)
  To: Nishka Dasgupta; +Cc: gregkh, w.d.hubbs, chris, kirk, outreachy-kernel

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.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
@ 2018-03-08 19:49 Nishka Dasgupta
  2018-03-08 19:53 ` Samuel Thibault
  0 siblings, 1 reply; 22+ messages in thread
From: Nishka Dasgupta @ 2018-03-08 19:49 UTC (permalink / raw)
  To: gregkh, w.d.hubbs, chris, kirk, samuel.thibault, outreachy-kernel
  Cc: 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



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
  2018-03-08 19:21 ` Nishka Dasgupta
@ 2018-03-08 19:34   ` Samuel Thibault
  0 siblings, 0 replies; 22+ messages in thread
From: Samuel Thibault @ 2018-03-08 19:34 UTC (permalink / raw)
  To: Nishka Dasgupta; +Cc: gregkh, w.d.hubbs, chris, kirk, outreachy-kernel

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
@ 2018-03-08 19:27 Nishka Dasgupta
  2018-03-08 19:21 ` Nishka Dasgupta
  0 siblings, 1 reply; 22+ messages in thread
From: Nishka Dasgupta @ 2018-03-08 19:27 UTC (permalink / raw)
  To: gregkh, w.d.hubbs, chris, kirk, samuel.thibault, outreachy-kernel
  Cc: 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



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
@ 2018-03-08 19:21 ` Nishka Dasgupta
  2018-03-08 19:34   ` Samuel Thibault
  0 siblings, 1 reply; 22+ messages in thread
From: Nishka Dasgupta @ 2018-03-08 19:21 UTC (permalink / raw)
  To: gregkh, w.d.hubbs, chris, kirk, samuel.thibault, outreachy-kernel
  Cc: 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



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] Staging: speakup: Replace simple_strtoul with kstrtoul
  2016-10-11  6:47 ` Greg Kroah-Hartman
  2016-10-11  7:29   ` Muraru Mihaela
@ 2016-10-11 19:49   ` Muraru Mihaela
  1 sibling, 0 replies; 22+ messages in thread
From: Muraru Mihaela @ 2016-10-11 19:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: outreachy-kernel

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Staging: speakup: Replace simple_strtoul with kstrtoul
  2016-10-11  6:47 ` Greg Kroah-Hartman
@ 2016-10-11  7:29   ` Muraru Mihaela
  2016-10-11 19:49   ` Muraru Mihaela
  1 sibling, 0 replies; 22+ messages in thread
From: Muraru Mihaela @ 2016-10-11  7:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: outreachy-kernel

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] Staging: speakup: Replace simple_strtoul with kstrtoul
  2016-10-11  6:18 [PATCH] Staging: " Mihaela Muraru
@ 2016-10-11  6:47 ` Greg Kroah-Hartman
  2016-10-11  7:29   ` Muraru Mihaela
  2016-10-11 19:49   ` Muraru Mihaela
  0 siblings, 2 replies; 22+ messages in thread
From: Greg Kroah-Hartman @ 2016-10-11  6:47 UTC (permalink / raw)
  To: Mihaela Muraru; +Cc: outreachy-kernel

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] Staging: speakup: Replace simple_strtoul with kstrtoul
@ 2016-10-11  6:18 Mihaela Muraru
  2016-10-11  6:47 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 22+ messages in thread
From: Mihaela Muraru @ 2016-10-11  6:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: outreachy-kernel

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



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul
@ 2016-02-22 12:18 Amitoj Kaur Chawla
  0 siblings, 0 replies; 22+ messages in thread
From: Amitoj Kaur Chawla @ 2016-02-22 12:18 UTC (permalink / raw)
  To: outreachy-kernel

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



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul
  2014-12-18 14:48       ` samuel kihahu
@ 2014-12-18 21:25         ` Dan Carpenter
  0 siblings, 0 replies; 22+ messages in thread
From: Dan Carpenter @ 2014-12-18 21:25 UTC (permalink / raw)
  To: samuel kihahu; +Cc: devel, gregkh, speakup, linux-kernel

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul
  2014-12-17 14:03     ` Dan Carpenter
@ 2014-12-18 14:48       ` samuel kihahu
  2014-12-18 21:25         ` Dan Carpenter
  0 siblings, 1 reply; 22+ messages in thread
From: samuel kihahu @ 2014-12-18 14:48 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: devel, gregkh, speakup, linux-kernel

[-- 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul
  2014-12-17 13:43   ` samuel kihahu
@ 2014-12-17 14:03     ` Dan Carpenter
  2014-12-18 14:48       ` samuel kihahu
  0 siblings, 1 reply; 22+ messages in thread
From: Dan Carpenter @ 2014-12-17 14:03 UTC (permalink / raw)
  To: samuel kihahu; +Cc: devel, gregkh, speakup, linux-kernel

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul
  2014-12-17 12:11 ` Dan Carpenter
@ 2014-12-17 13:43   ` samuel kihahu
  2014-12-17 14:03     ` Dan Carpenter
  0 siblings, 1 reply; 22+ messages in thread
From: samuel kihahu @ 2014-12-17 13:43 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: gregkh, devel, speakup, linux-kernel

[-- 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] staging: speakup: replace simple_strtoul with kstrtoul
  2014-12-17 11:56 [PATCH] staging: speakup: replace " samuel kihahu
@ 2014-12-17 12:11 ` Dan Carpenter
  2014-12-17 13:43   ` samuel kihahu
  0 siblings, 1 reply; 22+ messages in thread
From: Dan Carpenter @ 2014-12-17 12:11 UTC (permalink / raw)
  To: samuel kihahu; +Cc: gregkh, devel, speakup, linux-kernel

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] staging: speakup: replace simple_strtoul with kstrtoul
@ 2014-12-17 11:56 samuel kihahu
  2014-12-17 12:11 ` Dan Carpenter
  0 siblings, 1 reply; 22+ messages in thread
From: samuel kihahu @ 2014-12-17 11:56 UTC (permalink / raw)
  To: gregkh; +Cc: speakup, devel, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-02-23 20:02 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-08 19:43 [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul Nishka Dasgupta
2018-03-08 19:45 ` Samuel Thibault
2018-03-08 20:51 ` [Outreachy kernel] " Julia Lawall
  -- strict thread matches above, loose matches on Subject: below --
2019-02-23 19:31 Bharath Vedartham
2019-02-23 19:37 ` Samuel Thibault
2019-02-23 20:02   ` Bharath Vedartham
2018-03-08 19:49 Nishka Dasgupta
2018-03-08 19:53 ` Samuel Thibault
2018-03-08 19:27 Nishka Dasgupta
2018-03-08 19:21 ` Nishka Dasgupta
2018-03-08 19:34   ` Samuel Thibault
2016-10-11  6:18 [PATCH] Staging: " Mihaela Muraru
2016-10-11  6:47 ` Greg Kroah-Hartman
2016-10-11  7:29   ` Muraru Mihaela
2016-10-11 19:49   ` Muraru Mihaela
2016-02-22 12:18 [PATCH] staging: " Amitoj Kaur Chawla
2014-12-17 11:56 [PATCH] staging: speakup: replace " samuel kihahu
2014-12-17 12:11 ` Dan Carpenter
2014-12-17 13:43   ` samuel kihahu
2014-12-17 14:03     ` Dan Carpenter
2014-12-18 14:48       ` samuel kihahu
2014-12-18 21:25         ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.