[PATCH 0/3] checkpatch: Add support for decimal values

[PATCH 0/3] checkpatch: Add support for decimal values

[Joe Perches]

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(-)

-- 
1.7.8.112.g3fd21

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

[Joe Perches]

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.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f18750e..099a0ad 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -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{
-- 
1.7.8.112.g3fd21

[PATCH 2/3] checkpatch: Add support for decimal constants

[Joe Perches]

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

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 099a0ad..3e727c5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -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{
-- 
1.7.8.112.g3fd21

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

[Joe Perches]

Linux kernel doesn't like decimals, say so.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3e727c5..3339ecf 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -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) {
-- 
1.7.8.112.g3fd21

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

[Pavel Machek]

Hi!

> 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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Andrew Morton]

On Wed, 31 Oct 2012 11:37:03 +0100
Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > 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....

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.

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

[Pavel Machek]

Hi!

> > > 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....
> 
> 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;
....
> 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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Borislav Petkov]

On Wed, Oct 31, 2012 at 03:49:08PM -0700, Andrew Morton wrote:
> 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.

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.

-- 
Regards/Gruss,
    Boris.