Skip to contentSkip to conversation

checkpatch: Add support for decimal values

PATCH Superseded Joe Perches 4 participants 8 messages Oct 31, 2012 – Nov 1, 2012 Checkpatch

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Joe Perches

author maintainer
#0 lore raw

cover checkpatch: Add support for decimal values

The kernel doesn't use decimals though.

Joe Perches (3):
checkpatch: Find hex constants as a single IDENT
checkpatch: Add support for decimal constants
checkpatch: Emit a warning when decimal values are used

 scripts/checkpatch.pl |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
Signature · 1 line
1.7.8.112.g3fd21
author maintainer
#1 lore raw

PATCH 1/3 checkpatch: Find hex constants as a single IDENT

Hexadecimal values are currently found in 2 parts. A hex constant like 0x123456abcdef is found as 0 and then x123456abcdef and later coalesced.

Instead, reverse the order of the 2 searches in
$Constant to find 0x first, then 0 so that the
entire hex constant is found all at once.

---
 scripts/checkpatch.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
scripts/checkpatch.pl +1 -1
index f18750e..099a0ad 100755
@@ -227,7 +227,7 @@ our $Inline	= qr{inline|__always_inline|noinline};
 our $Member	= qr{->$Ident|\.$Ident|\[[^]]*\]};
 our $Lval	= qr{$Ident(?:$Member)*};
 
-our $Constant	= qr{(?i:(?:[0-9]+|0x[0-9a-f]+)[ul]*)};
+our $Constant	= qr{(?i:(?:0x[0-9a-f]+|[0-9]+)[ul]*)};
 our $Assignment	= qr{(?:\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=)};
 our $Compare    = qr{<=|>=|==|!=|<|>};
 our $Operators	= qr{
Signature · 1 line
1.7.8.112.g3fd21
author maintainer
#2 lore raw

PATCH 2/3 checkpatch: Add support for decimal constants

In reply to Joe Perches

Even though the kernel doesn't support using decimal constants, add a regex for them.

---
 scripts/checkpatch.pl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
scripts/checkpatch.pl +2 -1
index 099a0ad..3e727c5 100755
@@ -227,7 +227,8 @@ our $Inline	= qr{inline|__always_inline|noinline};
 our $Member	= qr{->$Ident|\.$Ident|\[[^]]*\]};
 our $Lval	= qr{$Ident(?:$Member)*};
 
-our $Constant	= qr{(?i:(?:0x[0-9a-f]+|[0-9]+)[ul]*)};
+our $Decimal	= qr{(?:(?:[0-9]+\.[0-9]*|[0-9]*\.[0-9]+))};
+our $Constant	= qr{(?:$Decimal|(?i:(?:0x[0-9a-f]+|[0-9]+)[ul]*))};
 our $Assignment	= qr{(?:\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=)};
 our $Compare    = qr{<=|>=|==|!=|<|>};
 our $Operators	= qr{
Signature · 1 line
1.7.8.112.g3fd21
author maintainer
#3 lore raw

PATCH 3/3 checkpatch: Emit a warning when decimal values are used

In reply to Joe Perches

Linux kernel doesn't like decimals, say so.

---
 scripts/checkpatch.pl |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
scripts/checkpatch.pl +7 -0
index 3e727c5..3339ecf 100755
@@ -2329,6 +2329,13 @@ sub process {
 			     "do not add new typedefs\n" . $herecurr);
 		}
 
+# check for decimal constants
+
+		if ($line =~ /\b$Decimal\b/) {
+			WARN("KERNEL_DECIMAL",
+			     "Decimal values are not supported in linux kernel source\n" . $herecurr);
+		}
+
 # * goes on variable not on type
 		# (char*[ const])
 		while ($line =~ m{(\($NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)\))}g) {
Signature · 1 line
1.7.8.112.g3fd21
#4 lore raw

Re: [PATCH 3/3] checkpatch: Emit a warning when decimal values are used

Hi!

Quoted 1 line

Linux kernel doesn't like decimals, say so.

?!

Linux surely supports decimal constants, like "100". Did you mean "octal"?

If you wanted to add warning for something... I never want to see

#define CRAPPY_EMBEDDED_REGISTER ((0x1) << (0))

again....
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#5 lore raw

Re: [PATCH 3/3] checkpatch: Emit a warning when decimal values are used

Quoted 6 lines · On Wed, 31 Oct 2012 11:37:03 +0100 Pavel Machek <[email protected]> wrote:

Hi!

Quoted 1 line

Linux kernel doesn't like decimals, say so.

?!

Quoted 8 lines

Linux surely supports decimal constants, like "100". Did you mean "octal"?

If you wanted to add warning for something... I never want to see

#define CRAPPY_EMBEDDED_REGISTER ((0x1) << (0))

again....

Joe means floating point. I suggest that the patchset be reworked, using s/decimal/float/g.

The kernel does have floating point constants, in various graphics drivers, iirc. They are used in places where the floatiness gets handled at complation time. Along the lines of:

	int foo = 1.1 * 2.2;

And I suppose that's an OK thing to do. We could instead do

	int foo = 2;	/* 1.1 * 2.2 */

but that's taking away a programmer convenience for no good reason. It would be highly inconvenient if the "1.1" was in fact a #define in some other file, or a Kconfig string.

That being said, I guess it's a worthwhile thing for checkpatch to warn about. Hopefully the programmer will say "well thanks, but I meant to do that".

A much better solution would be to arrange for the kernel to fail to compile (or to fail to link) if floats are used. That way, people could continue to use floats within their compile-time scalar expressions without getting harrassed by checkpatch. But I don't know how to arrange this.

hm.

#6 lore raw

Re: [PATCH 3/3] checkpatch: Emit a warning when decimal values are used

Hi!

Quoted 22 lines
Quoted 4 lines
Quoted 1 line

Linux kernel doesn't like decimals, say so.

?!

Quoted 8 lines

Linux surely supports decimal constants, like "100". Did you mean "octal"?

If you wanted to add warning for something... I never want to see

#define CRAPPY_EMBEDDED_REGISTER ((0x1) << (0))

again....

Joe means floating point. I suggest that the patchset be reworked, using s/decimal/float/g.

The kernel does have floating point constants, in various graphics drivers, iirc. They are used in places where the floatiness gets handled at complation time. Along the lines of:

	int foo = 1.1 * 2.2;

....

Quoted 5 lines

A much better solution would be to arrange for the kernel to fail to compile (or to fail to link) if floats are used. That way, people could continue to use floats within their compile-time scalar expressions without getting harrassed by checkpatch. But I don't know how to arrange this.

I know it is normally unwanted, but we do have kernel_fpu_begin(). I thought it enables exactly... floating point math in kernel?
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#7 lore raw

Re: [PATCH 3/3] checkpatch: Emit a warning when decimal values are used

In reply to Andrew Morton
Quoted 24 lines · Andrew Morton

Joe means floating point. I suggest that the patchset be reworked, using s/decimal/float/g.

The kernel does have floating point constants, in various graphics drivers, iirc. They are used in places where the floatiness gets handled at complation time. Along the lines of:

	int foo = 1.1 * 2.2;

And I suppose that's an OK thing to do. We could instead do

	int foo = 2;	/* 1.1 * 2.2 */

but that's taking away a programmer convenience for no good reason. It would be highly inconvenient if the "1.1" was in fact a #define in some other file, or a Kconfig string.

That being said, I guess it's a worthwhile thing for checkpatch to warn about. Hopefully the programmer will say "well thanks, but I meant to do that".

Quoted 5 lines

A much better solution would be to arrange for the kernel to fail to compile (or to fail to link) if floats are used. That way, people could continue to use floats within their compile-time scalar expressions without getting harrassed by checkpatch. But I don't know how to arrange this.

Run git grep on the source before compiling... Expensive though.

Also, Joe, I think you should add Andrew's explanation to the commit message so that it is clear what that check is for. Or maybe even as a comment above the check itself in checkpatch.pl.

Thanks.

Signature · 2 lines
Regards/Gruss,
    Boris.