[PATCH] checkpatch: Resolve improper warning for krealloc arg reuse

[PATCH] checkpatch: Resolve improper warning for krealloc arg reuse

[Manish Narani]

Correct the check for reuse of krealloc. It gives false warning when a
structure member variable name and krealloc argument name is same.

For Example:
{
	...
	abc.def_var = krealloc(def_var, sizeof(*def_var),
				    GFP_KERNEL);
	...
}

$ ./scripts/checkpatch.pl -f file.c

WARNING: Reusing the krealloc arg is almost always a bug
+	abc.def_var = krealloc(def_var, sizeof(*def_var),

This patch resolves the above false warning.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 447857f..db9b666 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6119,7 +6119,7 @@ sub process {
 
 # check for krealloc arg reuse
 		if ($^V && $^V ge 5.10.0 &&
-		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) {
+		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/ && $line !~ m/[\.|\-\>].*\s*\=\s*.*/) {
 			WARN("KREALLOC_ARG_REUSE",
 			     "Reusing the krealloc arg is almost always a bug\n" . $herecurr);
 		}
-- 
2.1.1

Re: [PATCH] checkpatch: Resolve improper warning for krealloc arg reuse

[Joe Perches]

On Wed, 2018-07-18 at 19:25 +0530, Manish Narani wrote:
> Correct the check for reuse of krealloc. It gives false warning when a
> structure member variable name and krealloc argument name is same.
> 
> For Example:
> {
> 	...
> 	abc.def_var = krealloc(def_var, sizeof(*def_var),
> 				    GFP_KERNEL);
> 	...
> }
> 
> $ ./scripts/checkpatch.pl -f file.c
> 
> WARNING: Reusing the krealloc arg is almost always a bug
> +	abc.def_var = krealloc(def_var, sizeof(*def_var),
> 
> This patch resolves the above false warning.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  scripts/checkpatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -6119,7 +6119,7 @@ sub process {
>  
>  # check for krealloc arg reuse
>  		if ($^V && $^V ge 5.10.0 &&
> -		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) {
> +		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/ && $line !~ m/[\.|\-\>].*\s*\=\s*.*/) {

Thanks but nack.
This is more an issue with $Lval.
I'll look around for a solution.

Re: [PATCH] checkpatch: Resolve improper warning for krealloc arg reuse

[Joe Perches]

On Wed, 2018-07-18 at 19:25 +0530, Manish Narani wrote:
> Correct the check for reuse of krealloc. It gives false warning when a
> structure member variable name and krealloc argument name is same.
> 
> For Example:
> {
> 	...
> 	abc.def_var = krealloc(def_var, sizeof(*def_var),
> 				    GFP_KERNEL);
> 	...
> }
> 
> $ ./scripts/checkpatch.pl -f file.c
> 
> WARNING: Reusing the krealloc arg is almost always a bug
> +	abc.def_var = krealloc(def_var, sizeof(*def_var),
> 
> This patch resolves the above false warning.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  scripts/checkpatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 447857f..db9b666 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6119,7 +6119,7 @@ sub process {
>  
>  # check for krealloc arg reuse
>  		if ($^V && $^V ge 5.10.0 &&
> -		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) {
> +		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/ && $line !~ m/[\.|\-\>].*\s*\=\s*.*/) {
>  			WARN("KREALLOC_ARG_REUSE",
>  			     "Reusing the krealloc arg is almost always a bug\n" . $herecurr);
>  		}

This works, though I'm not completely sure why.

Andy or anyone else:  Does your perl-foo understand why?

$ cat test_krealloc.c
static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np,
	unsigned int *conf)
{
	indio_dev->channels = krealloc(channels, sizeof(*channels) *
					num_channels, GFP_KERNEL);
	indio_dev->channels = krealloc(indio_dev->channels, sizeof(*channels) *
					num_channels, GFP_KERNEL);
}

For the patch below, the old case matches the first
$Lval above as "channels" skipping the "indio_dev->"
bit and as not "indio_dev->channels"

The modified patch below matches the first $Lval as the
expected "indio_dev->channels"
---
 scripts/checkpatch.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ed4fa986e8da..59eaaede757a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6149,7 +6149,8 @@ sub process {
 
 # check for krealloc arg reuse
 		if ($perl_version_ok &&
-		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) {
+		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*($Lval)\s*,/ &&
+		    $1 eq $3) {
 			WARN("KREALLOC_ARG_REUSE",
 			     "Reusing the krealloc arg is almost always a bug\n" . $herecurr);
 		}

[PATCH] checkpatch: Fix krealloc reuse test

[Joe Perches]

The current krealloc test does not function correctly when the
temporary pointer return name contains the original pointer name.

Fix that by maximally matching the return pointer name and the original
pointer name and doing a separate comparison of the both names.

Reported-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Joe Perches <joe@perches.com>
CC: Manish Narani <manish.narani@xilinx.com>
---
 scripts/checkpatch.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ed4fa986e8da..59eaaede757a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6149,7 +6149,8 @@ sub process {
 
 # check for krealloc arg reuse
 		if ($perl_version_ok &&
-		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) {
+		    $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*($Lval)\s*,/ &&
+		    $1 eq $3) {
 			WARN("KREALLOC_ARG_REUSE",
 			     "Reusing the krealloc arg is almost always a bug\n" . $herecurr);
 		}