| Message ID | ZQCaO+tYycDxVLy7@work (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [next] checkpatch: add a couple new alloc functions to alloc with multiplies check | expand |
On Tue, 2023-09-12 at 11:04 -0600, Gustavo A. R. Silva 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. > > Link: https://github.com/KSPP/linux/issues/342 > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > scripts/checkpatch.pl | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 7d16f863edf1..45265d0eee1b 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -7207,17 +7207,19 @@ 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 (kv|k|v)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/vmalloc_array/kvcalloc/kcalloc/vcalloc > 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*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,?/) { > my $oldfunc = $3; > my $a1 = $4; > my $a2 = $10; > my $newfunc = "kmalloc_array"; > $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc"); > + $newfunc = "vmalloc_array" if ($oldfunc eq "vmalloc"); > $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc"); > $newfunc = "kcalloc" if ($oldfunc eq "kzalloc"); > + $newfunc = "vcalloc" if ($oldfunc eq "vzalloc"); > my $r1 = $a1; > my $r2 = $a2; > if ($a1 =~ /^sizeof\s*\S/) { > @@ -7233,7 +7235,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*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; > } > } > } Seems ok. Dunno how many more of these there might be like devm_ variants, but maybe it'd be better style to use a hash with replacement instead of the repeated if block Something like: --- scripts/checkpatch.pl | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7d16f863edf1c..bacb63518be14 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -854,6 +854,23 @@ foreach my $entry (keys %deprecated_apis) { } $deprecated_apis_search = "(?:${deprecated_apis_search})"; +our %alloc_with_multiply_apis = ( + "kmalloc" => "kmalloc_array", + "kvmalloc" => "kvmalloc_array", + "vmalloc" => "vmalloc_array", + "kvzalloc" => "kvcalloc", + "kzalloc" => "kcalloc", + "vzalloc" => "vcalloc", +); + +#Create a search pattern for all these strings to speed up a loop below +our $alloc_with_multiply_search = ""; +foreach my $entry (keys %alloc_with_multiply_apis) { + $alloc_with_multiply_search .= '|' if ($alloc_with_multiply_search ne ""); + $alloc_with_multiply_search .= $entry; +} +$alloc_with_multiply_search = "(?:${alloc_with_multiply_search})"; + our $mode_perms_world_writable = qr{ S_IWUGO | S_IWOTH | @@ -7210,14 +7227,11 @@ sub process { # check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc 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/) { @@ -7233,7 +7247,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*($alloc_with_multiply_search)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; } } }
On Tue, Sep 12, 2023 at 10:51:22AM -0700, Joe Perches wrote: > On Tue, 2023-09-12 at 11:04 -0600, Gustavo A. R. Silva 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. > > > > Link: https://github.com/KSPP/linux/issues/342 > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > --- > > scripts/checkpatch.pl | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 7d16f863edf1..45265d0eee1b 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -7207,17 +7207,19 @@ 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 (kv|k|v)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/vmalloc_array/kvcalloc/kcalloc/vcalloc > > 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*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,?/) { > > my $oldfunc = $3; > > my $a1 = $4; > > my $a2 = $10; > > my $newfunc = "kmalloc_array"; > > $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc"); > > + $newfunc = "vmalloc_array" if ($oldfunc eq "vmalloc"); > > $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc"); > > $newfunc = "kcalloc" if ($oldfunc eq "kzalloc"); > > + $newfunc = "vcalloc" if ($oldfunc eq "vzalloc"); > > my $r1 = $a1; > > my $r2 = $a2; > > if ($a1 =~ /^sizeof\s*\S/) { > > @@ -7233,7 +7235,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*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; > > } > > } > > } > > Seems ok. Dunno how many more of these there might be like > devm_ variants, but maybe it'd be better style to use a hash > with replacement instead of the repeated if block > > Something like: > --- > scripts/checkpatch.pl | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 7d16f863edf1c..bacb63518be14 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -854,6 +854,23 @@ foreach my $entry (keys %deprecated_apis) { > } > $deprecated_apis_search = "(?:${deprecated_apis_search})"; > > +our %alloc_with_multiply_apis = ( > + "kmalloc" => "kmalloc_array", > + "kvmalloc" => "kvmalloc_array", > + "vmalloc" => "vmalloc_array", > + "kvzalloc" => "kvcalloc", > + "kzalloc" => "kcalloc", > + "vzalloc" => "vcalloc", > +); Yeah, this seems more readable. Gustavo can you send a v2 with this rework? Thanks! -Kees > + > +#Create a search pattern for all these strings to speed up a loop below > +our $alloc_with_multiply_search = ""; > +foreach my $entry (keys %alloc_with_multiply_apis) { > + $alloc_with_multiply_search .= '|' if ($alloc_with_multiply_search ne ""); > + $alloc_with_multiply_search .= $entry; > +} > +$alloc_with_multiply_search = "(?:${alloc_with_multiply_search})"; > + > our $mode_perms_world_writable = qr{ > S_IWUGO | > S_IWOTH | > @@ -7210,14 +7227,11 @@ sub process { > # check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc > 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/) { > @@ -7233,7 +7247,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*($alloc_with_multiply_search)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; > } > } > } >
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7d16f863edf1..45265d0eee1b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -7207,17 +7207,19 @@ 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 (kv|k|v)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/vmalloc_array/kvcalloc/kcalloc/vcalloc 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*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,?/) { my $oldfunc = $3; my $a1 = $4; my $a2 = $10; my $newfunc = "kmalloc_array"; $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc"); + $newfunc = "vmalloc_array" if ($oldfunc eq "vmalloc"); $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc"); $newfunc = "kcalloc" if ($oldfunc eq "kzalloc"); + $newfunc = "vcalloc" if ($oldfunc eq "vzalloc"); my $r1 = $a1; my $r2 = $a2; if ($a1 =~ /^sizeof\s*\S/) { @@ -7233,7 +7235,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*((?:kv|k|v)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; } } }
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. Link: https://github.com/KSPP/linux/issues/342 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- scripts/checkpatch.pl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)