checkpatch: kstrtol fix

checkpatch: kstrtol fix

[Joe Korty]

kstrtol is a substitute for simple_strtol() only when when second
arg of simple_strtol() is NULL. For any other value the functionality
of simple_strtol() cannot be implemented in terms of kstrtol.

So modify checkpatch.pl so that it prints out the following warning only
if the second argument is null:

   WARNING: simple_strtol is obsolete, use kstrtol instead

Signed-off-by: Joe Korty <joe.korty@ccur.com>

Index: linux/scripts/checkpatch.pl
===================================================================
--- linux.orig/scripts/checkpatch.pl	2012-05-02 10:38:45.000000000 -0400
+++ linux/scripts/checkpatch.pl	2012-05-02 10:47:58.000000000 -0400
@@ -3359,8 +3359,8 @@
 			     "consider using a completion\n" . $herecurr);
 		}
 
-# recommend kstrto* over simple_strto* and strict_strto*
-		if ($line =~ /\b((simple|strict)_(strto(l|ll|ul|ull)))\s*\(/) {
+# recommend kstrto* over simple_strto* and strict_strto* where appropriate
+		if ($line =~ /\b((simple|strict)_(strto(l|ll|ul|ull)))\s*\([^,]+,\s*NULL\s*,/) {
 			WARN("CONSIDER_KSTRTO",
 			     "$1 is obsolete, use k$3 instead\n" . $herecurr);
 		}

Re: checkpatch: kstrtol fix

[Guenter Roeck]

On Wed, 2012-05-02 at 11:10 -0400, Joe Korty wrote:
> kstrtol is a substitute for simple_strtol() only when when second
> arg of simple_strtol() is NULL. For any other value the functionality
> of simple_strtol() cannot be implemented in terms of kstrtol.
> 
> So modify checkpatch.pl so that it prints out the following warning only
> if the second argument is null:
> 
>    WARNING: simple_strtol is obsolete, use kstrtol instead
> 
> Signed-off-by: Joe Korty <joe.korty@ccur.com>
> 

I think this would be a bad idea. Most if not all instances in the
kernel (at least all the ones I looked at) use the second argument to
determine if the parameter was actually a number or not, ie it is used
to detect parameter errors. This is exactly the point of using kstrtol()
in the first place. So it _is_ possible, at least in most cases, to
implement the same functionality with kstrtol(). If there _are_ uses of
simple_strtol() where the second argument is not used for error checking
but for some other purpose, I am sure that an alternative solution can
be found which does not require simple_strtol(). Or just live with the
warning for those cases.

Thanks,
Guenter

Re: checkpatch: kstrtol fix

[Joe Korty]

On Wed, May 02, 2012 at 12:13:22PM -0400, Guenter Roeck wrote:
> On Wed, 2012-05-02 at 11:10 -0400, Joe Korty wrote:
>> kstrtol is a substitute for simple_strtol() only when when second
>> arg of simple_strtol() is NULL. For any other value the functionality
>> of simple_strtol() cannot be implemented in terms of kstrtol.
> 
> If there _are_ uses of
> simple_strtol() where the second argument is not used for error checking
> but for some other purpose, I am sure that an alternative solution can
> be found which does not require simple_strtol(). Or just live with the
> warning for those cases.
> 
> Thanks,
> Guenter

And of course the burning question is, what was the compelling reason
kstrtol wasn't made upwards-compatible with strtol?  It just seems
gratituous to have that little bit of incompatibility introduced
into the kernel.

Joe