Messages in this thread | | | Date | Sun, 31 May 2015 19:38:01 +0200 | From | Filip Ayazi <> | Subject | Re: [PATCH] kdb: use kstrto instead of simple_strto |
| |
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.
| |