2021-05-28 00:07:52

by Antonio Borneo

[permalink] [raw]
Subject: [PATCH] checkpatch: fix incorrect camelcase detection on numeric constant

The code fragment below
int foo(int *array, int index)
{
return array[index & 0xFF];
}
triggers an incorrect camelcase detection by checking a subset of
the hex constant:
CHECK: Avoid CamelCase: <xFF>
#3: FILE: test.c:3:
+ return array[index & 0xFF];

This is caused by passing the whole string "array[index & 0xFF]"
to the inner loop that iterates over a "$Ident" match.
With no check to exclude a constant, the match iterates over
"array", "index" and "xFF", thus the camelcase detection.

Similar issue can be detected with other constants like "1uL",
"0xffffU".

Force the match to start at word boundary so the constants will be
filtered-out.

Signed-off-by: Antonio Borneo <[email protected]>
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

To: Andy Whitcroft <[email protected]>
To: Joe Perches <[email protected]>
To: Dwaipayan Ray <[email protected]>
To: Lukas Bulwahn <[email protected]>
Cc: [email protected]

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 23697a6b1eaa..f0032166dfc1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5656,7 +5656,7 @@ sub process {
$var !~ /^(?:[a-z0-9_]*|[A-Z0-9_]*)?_?[a-z][A-Z](?:_[a-z0-9_]+|_[A-Z0-9_]+)?$/ &&
#Ignore some three character SI units explicitly, like MiB and KHz
$var !~ /^(?:[a-z_]*?)_?(?:[KMGT]iB|[KMGT]?Hz)(?:_[a-z_]+)?$/) {
- while ($var =~ m{($Ident)}g) {
+ while ($var =~ m{\b($Ident)}g) {
my $word = $1;
next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/);
if ($check) {

base-commit: c4681547bcce777daf576925a966ffa824edd09d
--
2.31.1


2021-05-28 02:52:15

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: fix incorrect camelcase detection on numeric constant

On Thu, 2021-05-27 at 23:47 +0200, Antonio Borneo wrote:
> The code fragment below
> int foo(int *array, int index)
> {
> return array[index & 0xFF];
> }
> triggers an incorrect camelcase detection by checking a subset of
> the hex constant:
> CHECK: Avoid CamelCase: <xFF>
> #3: FILE: test.c:3:
> + return array[index & 0xFF];
>
> This is caused by passing the whole string "array[index & 0xFF]"
> to the inner loop that iterates over a "$Ident" match.
> With no check to exclude a constant, the match iterates over
> "array", "index" and "xFF", thus the camelcase detection.

The thing there is that 0xFF is not an $Ident as it doesn't
start with _[A-Za-z] so it's not excluding constants per-se.

So, you've added the right code, but just not quite the best
commit message.

> Force the match to start at word boundary so the constants will be
> filtered-out.

Anyway, thanks and cheers, Joe

>
> Signed-off-by: Antonio Borneo <[email protected]>
> ---
> ?scripts/checkpatch.pl | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
> To: Andy Whitcroft <[email protected]>
> To: Joe Perches <[email protected]>
> To: Dwaipayan Ray <[email protected]>
> To: Lukas Bulwahn <[email protected]>
> Cc: [email protected]
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 23697a6b1eaa..f0032166dfc1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5656,7 +5656,7 @@ sub process {
> ? $var !~ /^(?:[a-z0-9_]*|[A-Z0-9_]*)?_?[a-z][A-Z](?:_[a-z0-9_]+|_[A-Z0-9_]+)?$/ &&
> ?#Ignore some three character SI units explicitly, like MiB and KHz
> ? $var !~ /^(?:[a-z_]*?)_?(?:[KMGT]iB|[KMGT]?Hz)(?:_[a-z_]+)?$/) {
> - while ($var =~ m{($Ident)}g) {
> + while ($var =~ m{\b($Ident)}g) {
> ? my $word = $1;
> ? next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/);
> ? if ($check) {
>
> base-commit: c4681547bcce777daf576925a966ffa824edd09d


2022-06-13 15:46:06

by Antonio Borneo

[permalink] [raw]
Subject: [PATCH v2] checkpatch: fix incorrect camelcase detection on numeric constant

The code fragment below
int foo(int *array, int index)
{
return array[index & 0xFF];
}
triggers an incorrect camelcase detection by checking a substring
of the hex constant:
CHECK: Avoid CamelCase: <xFF>
#3: FILE: test.c:3:
+ return array[index & 0xFF];

This is caused by passing the whole string "array[index & 0xFF]"
to the inner loop that iterates over a "$Ident" match.
The numeric constant is not a $Ident as it doesn't start with
[A-Za-z_] and should be excluded from the match.

Similar issue can be detected with other constants like "1uL",
"0xffffU".

Force the match to start at word boundary so the $Ident will be
properly checked starting from its first char and the constants
will be filtered-out.

Signed-off-by: Antonio Borneo <[email protected]>
---

Well, already one year has passed from v1, but anyway here is v2.

Joe,
I have modified the commit message, hope it fully matches your
review.

v1 -> v2:
fix description in commit message
rebase on v5.19-rc2

---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 503e8abbb2c1..ef4c656a99b3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5720,7 +5720,7 @@ sub process {
$var !~ /^(?:[a-z0-9_]*|[A-Z0-9_]*)?_?[a-z][A-Z](?:_[a-z0-9_]+|_[A-Z0-9_]+)?$/ &&
#Ignore some three character SI units explicitly, like MiB and KHz
$var !~ /^(?:[a-z_]*?)_?(?:[KMGT]iB|[KMGT]?Hz)(?:_[a-z_]+)?$/) {
- while ($var =~ m{($Ident)}g) {
+ while ($var =~ m{\b($Ident)}g) {
my $word = $1;
next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/);
if ($check) {

base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
--
2.36.1