Skip to contentSkip to conversation

checkpatch: kstrtol fix

PATCH Joe Korty 2 participants 3 messages May 2, 2012 Checkpatch

lore
Share & tools
lore

Joe Korty

author
#0 lore raw

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

scripts/checkpatch.pl +2 -2
Index: linux/scripts/checkpatch.pl
===================================================================
@@ -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);
 		}
#1 lore raw
Quoted 11 lines · 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

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

author
#2 lore raw
Quoted 5 lines · Guenter Roeck
Quoted 3 lines · 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.

Quoted 8 lines

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