* Re: [PATCH] checkpatch: Resolve improper warning for krealloc arg reuse
2018-07-18 13:55 [PATCH] checkpatch: Resolve improper warning for krealloc arg reuse Manish Narani
@ 2018-07-18 15:50 ` Joe Perches
2018-07-18 17:46 ` Joe Perches
2018-07-22 17:51 ` [PATCH] checkpatch: Fix krealloc reuse test Joe Perches
2 siblings, 0 replies; 4+ messages in thread
From: Joe Perches @ 2018-07-18 15:50 UTC (permalink / raw)
To: Manish Narani, apw, linux-kernel; +Cc: anirudh, sgoud, svemula
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.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] checkpatch: Resolve improper warning for krealloc arg reuse
2018-07-18 13:55 [PATCH] checkpatch: Resolve improper warning for krealloc arg reuse Manish Narani
2018-07-18 15:50 ` Joe Perches
@ 2018-07-18 17:46 ` Joe Perches
2018-07-22 17:51 ` [PATCH] checkpatch: Fix krealloc reuse test Joe Perches
2 siblings, 0 replies; 4+ messages in thread
From: Joe Perches @ 2018-07-18 17:46 UTC (permalink / raw)
To: Manish Narani, apw, linux-kernel; +Cc: anirudh, sgoud, svemula
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);
}
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] checkpatch: Fix krealloc reuse test
2018-07-18 13:55 [PATCH] checkpatch: Resolve improper warning for krealloc arg reuse Manish Narani
2018-07-18 15:50 ` Joe Perches
2018-07-18 17:46 ` Joe Perches
@ 2018-07-22 17:51 ` Joe Perches
2 siblings, 0 replies; 4+ messages in thread
From: Joe Perches @ 2018-07-22 17:51 UTC (permalink / raw)
To: Andrew Morton, Andy Whitcroft
Cc: anirudh, sgoud, svemula, Lars-Peter Clausen, Manish Narani, LKML
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);
}
^ permalink raw reply [flat|nested] 4+ messages in thread