[PATCH] kdb: use kstrto instead of simple_strto

[PATCH] kdb: use kstrto instead of simple_strto

[Filip Ayazi]

Replace obsolete simple_strto* calls with appropriate kstrto* functions.

Signed-off-by: Filip Ayazi <filipayazi@gmail.com>
---
 kernel/debug/kdb/kdb_main.c | 53 +++++++++++++++------------------------------
 1 file changed, 17 insertions(+), 36 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4121345..0e0371c 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -296,7 +296,8 @@ static int kdbgetulenv(const char *match, unsigned long *value)
 	if (strlen(ep) == 0)
 		return KDB_NOENVVALUE;
 
-	*value = simple_strtoul(ep, NULL, 0);
+	if (kstrtoul(ep, 0, value) != 0)
+		return KDB_BADINT;
 
 	return 0;
 }
@@ -334,20 +335,15 @@ int kdbgetintenv(const char *match, int *value)
  */
 int kdbgetularg(const char *arg, unsigned long *value)
 {
-	char *endp;
 	unsigned long val;
 
-	val = simple_strtoul(arg, &endp, 0);
-
-	if (endp == arg) {
-		/*
-		 * Also try base 16, for us folks too lazy to type the
-		 * leading 0x...
-		 */
-		val = simple_strtoul(arg, &endp, 16);
-		if (endp == arg)
+	/*
+	 * Also try base 16, for us folks too lazy to type the
+	 * leading 0x...
+	 */
+	if (kstrtoul(arg, 0, &val) != 0)
+		if (kstrtoul(arg, 16, &val) != 0)
 			return KDB_BADINT;
-	}
 
 	*value = val;
 
@@ -356,17 +352,11 @@ int kdbgetularg(const char *arg, unsigned long *value)
 
 int kdbgetu64arg(const char *arg, u64 *value)
 {
-	char *endp;
 	u64 val;
 
-	val = simple_strtoull(arg, &endp, 0);
-
-	if (endp == arg) {
-
-		val = simple_strtoull(arg, &endp, 16);
-		if (endp == arg)
+	if (kstrtoull(arg, 0, &val) != 0)
+		if (kstrtoull(arg, 16, &val) != 0)
 			return KDB_BADINT;
-	}
 
 	*value = val;
 
@@ -402,10 +392,9 @@ int kdb_set(int argc, const char **argv)
 	 */
 	if (strcmp(argv[1], "KDBDEBUG") == 0) {
 		unsigned int debugflags;
-		char *cp;
 
-		debugflags = simple_strtoul(argv[2], &cp, 0);
-		if (cp == argv[2] || debugflags & ~KDB_DEBUG_FLAG_MASK) {
+		if (kstrtouint(argv[2], 0, &debugflags) != 0
+		    || debugflags & ~KDB_DEBUG_FLAG_MASK) {
 			kdb_printf("kdb: illegal debug flags '%s'\n",
 				    argv[2]);
 			return 0;
@@ -1588,10 +1577,8 @@ static int kdb_md(int argc, const char **argv)
 		if (!argv[0][3])
 			valid = 1;
 		else if (argv[0][3] == 'c' && argv[0][4]) {
-			char *p;
-			repeat = simple_strtoul(argv[0] + 4, &p, 10);
+			valid = !kstrtoint(argv[0]+4, 10, &repeat);
 			mdcount = ((repeat * bytesperword) + 15) / 16;
-			valid = !*p;
 		}
 		last_repeat = repeat;
 	} else if (strcmp(argv[0], "md") == 0)
@@ -2091,13 +2078,10 @@ static int kdb_dmesg(int argc, const char **argv)
 	if (argc > 2)
 		return KDB_ARGCOUNT;
 	if (argc) {
-		char *cp;
-		lines = simple_strtol(argv[1], &cp, 0);
-		if (*cp)
+		if (kstrtoint(argv[1], 0, &lines) != 0)
 			lines = 0;
 		if (argc > 1) {
-			adjust = simple_strtoul(argv[2], &cp, 0);
-			if (*cp || adjust < 0)
+			if (kstrtoint(argv[2], 0, &adjust) != 0 || adjust < 0)
 				adjust = 0;
 		}
 	}
@@ -2437,15 +2421,13 @@ static int kdb_help(int argc, const char **argv)
 static int kdb_kill(int argc, const char **argv)
 {
 	long sig, pid;
-	char *endp;
 	struct task_struct *p;
 	struct siginfo info;
 
 	if (argc != 2)
 		return KDB_ARGCOUNT;
 
-	sig = simple_strtol(argv[1], &endp, 0);
-	if (*endp)
+	if (kstrtol(argv[1], 0, &sig) != 0)
 		return KDB_BADINT;
 	if (sig >= 0) {
 		kdb_printf("Invalid signal parameter.<-signal>\n");
@@ -2453,8 +2435,7 @@ static int kdb_kill(int argc, const char **argv)
 	}
 	sig = -sig;
 
-	pid = simple_strtol(argv[2], &endp, 0);
-	if (*endp)
+	if (kstrtol(argv[2], 0, &pid) != 0)
 		return KDB_BADINT;
 	if (pid <= 0) {
 		kdb_printf("Process ID must be large than 0.\n");
-- 
1.9.1

Re: [PATCH] kdb: use kstrto instead of simple_strto

[Alexey Dobriyan]

> Replace obsolete simple_strto* calls with appropriate kstrto*
> functions.

>  static int kdb_kill(int argc, const char **argv)
>  {
>  	long sig, pid;
> -	char *endp;
>  	struct task_struct *p;
>  	struct siginfo info;
> 
>  	if (argc != 2)
>  		return KDB_ARGCOUNT;
> 
> -	sig = simple_strtol(argv[1], &endp, 0);
> -	if (*endp)
> +	if (kstrtol(argv[1], 0, &sig) != 0)
>  		return KDB_BADINT;
>  	if (sig >= 0) {
>  		kdb_printf("Invalid signal parameter.<-signal>\n");
>  <at>  <at>  -2453,8 +2435,7  <at>  <at>  static int kdb_kill(int argc, const char **argv)
>  	}
>  	sig = -sig;
> 
> -	pid = simple_strtol(argv[2], &endp, 0);
> -	if (*endp)
> +	if (kstrtol(argv[2], 0, &pid) != 0)
>  		return KDB_BADINT;
>  	if (pid <= 0) {
>  		kdb_printf("Process ID must be large than 0.\n");

This patch does easy mistake, namely trivial switch from simple_strtoul() to kstrtoul().

But you have to remember that there are NO simple_strtoint() or something like that,
so real type of data is anchor not function name.

In the code above data are "sig" and "pid" which are "int" or "unsigned int" at most.
Later "sig" is negated, so "int sig" is OK.

Pids are always unsigned logically so "unsigned int" is correct.

Don't be afraid to switch type of variable to correct one. Most of
the time "unsigned long foo" is wrong becase C-derived api didn't offer
anything better.

Re: [PATCH] kdb: use kstrto instead of simple_strto

[Filip Ayazi]

On 05/31/2015 02:29 PM, Alexey Dobriyan wrote:
>> Replace obsolete simple_strto* calls with appropriate kstrto*
>> functions.
>>   static int kdb_kill(int argc, const char **argv)
>>   {
>>   	long sig, pid;
>> -	char *endp;
>>   	struct task_struct *p;
>>   	struct siginfo info;
>>
>>   	if (argc != 2)
>>   		return KDB_ARGCOUNT;
>>
>> -	sig = simple_strtol(argv[1], &endp, 0);
>> -	if (*endp)
>> +	if (kstrtol(argv[1], 0, &sig) != 0)
>>   		return KDB_BADINT;
>>   	if (sig >= 0) {
>>   		kdb_printf("Invalid signal parameter.<-signal>\n");
>>   <at>  <at>  -2453,8 +2435,7  <at>  <at>  static int kdb_kill(int argc, const char **argv)
>>   	}
>>   	sig = -sig;
>>
>> -	pid = simple_strtol(argv[2], &endp, 0);
>> -	if (*endp)
>> +	if (kstrtol(argv[2], 0, &pid) != 0)
>>   		return KDB_BADINT;
>>   	if (pid <= 0) {
>>   		kdb_printf("Process ID must be large than 0.\n");
> This patch does easy mistake, namely trivial switch from simple_strtoul() to kstrtoul().
>
> But you have to remember that there are NO simple_strtoint() or something like that,
> so real type of data is anchor not function name.
>
> In the code above data are "sig" and "pid" which are "int" or "unsigned int" at most.
> Later "sig" is negated, so "int sig" is OK.

Thank you for the feedback, I know there is no simple_strtoint and the patch
uses kstrtoint/uint where the variable is declared as int/unsigned int 
(not doing
so would cause compilation warnings).
> Pids are always unsigned logically so "unsigned int" is correct.
>
> Don't be afraid to switch type of variable to correct one. Most of
> the time "unsigned long foo" is wrong becase C-derived api didn't offer
> anything better.

pid and sig variables are declared as long, I didn't know why so I 
didn't change
that, I will change their types to uint and int as you said and resend 
the patch soon.