[2/2] checkpatch: Add a couple new alloc functions to alloc with multiplies check

Message ID edb667e19211652a32ef6069159bb85dbc3bcdfc.1694636817.git.joe@perches.com
State New
Headers
Series checkpatch: refactor and add new alloc w/ multiply tests |

Commit Message

Joe Perches Sept. 13, 2023, 8:37 p.m. UTC
  vmalloc() and vzalloc() functions have now 2-factor multiplication
argument forms vmalloc_array() and vcalloc(), correspondingly.

Add alloc-with-multiplies checks for these new functions.

Simplify the original codes repeated else to use a hash.

Link: https://github.com/KSPP/linux/issues/342

Original-patch-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Link: https://lore.kernel.org/lkml/ZQCaO+tYycDxVLy7@work/
Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)
  

Comments

Gustavo A. R. Silva Sept. 13, 2023, 10:14 p.m. UTC | #1
On 9/13/23 14:37, Joe Perches wrote:
> vmalloc() and vzalloc() functions have now 2-factor multiplication
> argument forms vmalloc_array() and vcalloc(), correspondingly.
> 
> Add alloc-with-multiplies checks for these new functions.
> 
> Simplify the original codes repeated else to use a hash.
> 
> Link: https://github.com/KSPP/linux/issues/342
> 
> Original-patch-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Why don't you wait for a response or a v2 from the original
submitter?

--
Gustavo
  
Joe Perches Sept. 13, 2023, 10:25 p.m. UTC | #2
On Wed, 2023-09-13 at 16:14 -0600, Gustavo A. R. Silva wrote:
> 
> On 9/13/23 14:37, Joe Perches wrote:
> > vmalloc() and vzalloc() functions have now 2-factor multiplication
> > argument forms vmalloc_array() and vcalloc(), correspondingly.
> > 
> > Add alloc-with-multiplies checks for these new functions.
> > 
> > Simplify the original codes repeated else to use a hash.
> > 
> > Link: https://github.com/KSPP/linux/issues/342
> > 
> > Original-patch-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> 
> Why don't you wait for a response or a v2 from the original
> submitter?

Because there really is no need to wait.
  
Gustavo A. R. Silva Sept. 13, 2023, 10:32 p.m. UTC | #3
On 9/13/23 16:25, Joe Perches wrote:
> On Wed, 2023-09-13 at 16:14 -0600, Gustavo A. R. Silva wrote:
>>
>> On 9/13/23 14:37, Joe Perches wrote:
>>> vmalloc() and vzalloc() functions have now 2-factor multiplication
>>> argument forms vmalloc_array() and vcalloc(), correspondingly.
>>>
>>> Add alloc-with-multiplies checks for these new functions.
>>>
>>> Simplify the original codes repeated else to use a hash.
>>>
>>> Link: https://github.com/KSPP/linux/issues/342
>>>
>>> Original-patch-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>
>> Why don't you wait for a response or a v2 from the original
>> submitter?
> 
> Because there really is no need to wait.

By the way, did you test it?

--
Gustavo
  
Joe Perches Sept. 13, 2023, 10:34 p.m. UTC | #4
On Wed, 2023-09-13 at 16:32 -0600, Gustavo A. R. Silva wrote:
> 
> On 9/13/23 16:25, Joe Perches wrote:
> > On Wed, 2023-09-13 at 16:14 -0600, Gustavo A. R. Silva wrote:
> > > 
> > > On 9/13/23 14:37, Joe Perches wrote:
> > > > vmalloc() and vzalloc() functions have now 2-factor multiplication
> > > > argument forms vmalloc_array() and vcalloc(), correspondingly.
> > > > 
> > > > Add alloc-with-multiplies checks for these new functions.
> > > > 
> > > > Simplify the original codes repeated else to use a hash.
> > > > 
> > > > Link: https://github.com/KSPP/linux/issues/342
> > > > 
> > > > Original-patch-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > > 
> > > Why don't you wait for a response or a v2 from the original
> > > submitter?
> > 
> > Because there really is no need to wait.
> 
> By the way, did you test it?

Yes, against arch/s390/include/asm/idals.h

Did you?  If so, you could also add your own tested-by...
  
Gustavo A. R. Silva Sept. 13, 2023, 10:41 p.m. UTC | #5
On 9/13/23 16:34, Joe Perches wrote:
> On Wed, 2023-09-13 at 16:32 -0600, Gustavo A. R. Silva wrote:
>>
>> On 9/13/23 16:25, Joe Perches wrote:
>>> On Wed, 2023-09-13 at 16:14 -0600, Gustavo A. R. Silva wrote:
>>>>
>>>> On 9/13/23 14:37, Joe Perches wrote:
>>>>> vmalloc() and vzalloc() functions have now 2-factor multiplication
>>>>> argument forms vmalloc_array() and vcalloc(), correspondingly.
>>>>>
>>>>> Add alloc-with-multiplies checks for these new functions.
>>>>>
>>>>> Simplify the original codes repeated else to use a hash.
>>>>>
>>>>> Link: https://github.com/KSPP/linux/issues/342
>>>>>
>>>>> Original-patch-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>>>
>>>> Why don't you wait for a response or a v2 from the original
>>>> submitter?
>>>
>>> Because there really is no need to wait.
>>
>> By the way, did you test it?
> 
> Yes, against arch/s390/include/asm/idals.h

I don't see any instances of vmalloc() or vzalloc() in that file.

--
Gustavo
  
Joe Perches Sept. 13, 2023, 11 p.m. UTC | #6
On Wed, 2023-09-13 at 16:41 -0600, Gustavo A. R. Silva wrote:
> 
> On 9/13/23 16:34, Joe Perches wrote:
> > On Wed, 2023-09-13 at 16:32 -0600, Gustavo A. R. Silva wrote:
> > > 
> > > On 9/13/23 16:25, Joe Perches wrote:
> > > > On Wed, 2023-09-13 at 16:14 -0600, Gustavo A. R. Silva wrote:
> > > > > 
> > > > > On 9/13/23 14:37, Joe Perches wrote:
> > > > > > vmalloc() and vzalloc() functions have now 2-factor multiplication
> > > > > > argument forms vmalloc_array() and vcalloc(), correspondingly.
> > > > > > 
> > > > > > Add alloc-with-multiplies checks for these new functions.
> > > > > > 
> > > > > > Simplify the original codes repeated else to use a hash.
> > > > > > 
> > > > > > Link: https://github.com/KSPP/linux/issues/342
> > > > > > 
> > > > > > Original-patch-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > > > > 
> > > > > Why don't you wait for a response or a v2 from the original
> > > > > submitter?
> > > > 
> > > > Because there really is no need to wait.
> > > 
> > > By the way, did you test it?
> > 
> > Yes, against arch/s390/include/asm/idals.h
> 
> I don't see any instances of vmalloc() or vzalloc() in that file.

<snort>  Cute.

Missing a ? in the original search '\s*,?'
or just its removal...

Likely the removal would be better.

thanks, cheers, Joe
  

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 617f9e53bacdf..4cb248985eefc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -834,6 +834,16 @@  our %deprecated_apis = (
 #Create a search pattern for all these strings to speed up a loop below
 our $deprecated_apis_search = '(?:' . join('|', keys %deprecated_apis) . ')';
 
+our %alloc_with_multiply_apis = (
+	"kmalloc"		=> "kmalloc_array",
+	"kvmalloc"		=> "kvmalloc_array",
+	"vmalloc"		=> "vmalloc_array",
+	"kvzalloc"		=> "kvcalloc",
+	"kzalloc"		=> "kcalloc",
+	"vzalloc"		=> "vcalloc",
+);
+our $alloc_with_multiply_search = '(?:' . join('|', keys %alloc_with_multiply_apis) . ')';
+
 our $mode_perms_world_writable = qr{
 	S_IWUGO		|
 	S_IWOTH		|
@@ -7187,17 +7197,14 @@  sub process {
 			    "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr);
 		}
 
-# check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc
+# check for various allocs with multiplies that should use safer functions
 		if ($perl_version_ok &&
 		    defined $stat &&
-		    $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
+		    $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*($alloc_with_multiply_search)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) {
 			my $oldfunc = $3;
+			my $newfunc = $alloc_with_multiply_apis{$oldfunc};
 			my $a1 = $4;
 			my $a2 = $10;
-			my $newfunc = "kmalloc_array";
-			$newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc");
-			$newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc");
-			$newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
 			my $r1 = $a1;
 			my $r2 = $a2;
 			if ($a1 =~ /^sizeof\s*\S/) {
@@ -7213,7 +7220,7 @@  sub process {
 					 "Prefer $newfunc over $oldfunc with multiply\n" . $herectx) &&
 				    $cnt == 1 &&
 				    $fix) {
-					$fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
+					$fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*($oldfunc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
 				}
 			}
 		}