On Fri, 27 Jun 2014 14:37:04 -0700
Joe Perches <j...@perches.com> wrote:
> On Fri, 2014-06-27 at 22:10 +0200, Fabian Frederick wrote:
> > Avoid automatic k[mz]alloc with multiplies conversions
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -4427,7 +4427,7 @@ sub process {
> > $newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
> > if ($a1 =~ /^sizeof\s*\S/ || $a2 =~ /^sizeof\s*\S/) {
> > if (WARN("ALLOC_WITH_MULTIPLY",
> > - "Prefer $newfunc over $oldfunc with
> > multiply\n" . $herecurr) &&
> > + "Prefer $newfunc over $oldfunc with
> > multiply when arguments are not fixed or come from unvalidated source\n" .
> > $herecurr) &&
> > $fix) {
> > my $r1 = $a1;
> > my $r2 = $a2;
>
> I'm not sure of the value of this as I think at some point
> if not already today, the compiler will optimize the multiply
> away.
>
> But it's probably better to look at the non-sizeof variable
> and emit the warning only when it's not $Constant or some
> upper-case only macro #define like "\b[A-Z_]+\b" is used.
>
> Maybe something like this:
>
> scripts/checkpatch.pl | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 6c7cbaf..e0d0ead 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -4486,22 +4486,23 @@ sub process {
>
> # check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc
> if ($^V && $^V ge 5.10.0 &&
> - $line =~
> /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/)
> {
> + $line =~
> /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/)
> {
> my $oldfunc = $3;
> my $a1 = $4;
> my $a2 = $10;
> my $newfunc = "kmalloc_array";
> $newfunc = "kcalloc" if ($oldfunc eq "kzalloc");
> - if ($a1 =~ /^sizeof\s*\S/ || $a2 =~ /^sizeof\s*\S/) {
> + my $r1 = $a1;
> + my $r2 = $a2;
> + if ($a1 =~ /^sizeof\s*\S/) {
> + $r1 = $a2;
> + $r2 = $a1;
> + }
> + if ($r1 !~ /^sizeof\b/ && $r2 =~ /^sizeof\s*\S/ &&
> + !($r1 =~ /^$Constant$/ || $r1 =~ /^[A-Z_]+$/)) {
> if (WARN("ALLOC_WITH_MULTIPLY",
> "Prefer $newfunc over $oldfunc with
> multiply\n" . $herecurr) &&
> $fix) {
> - my $r1 = $a1;
> - my $r2 = $a2;
> - if ($a1 =~ /^sizeof\s*\S/) {
> - $r1 = $a2;
> - $r2 = $a1;
> - }
> $fixed[$linenr - 1] =~
> s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1
> . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e;
>
> }
>
>
Hello Joe,
Thanks, it looks great. Already tested ? If not I can do it and give you
some feedback ...
Fabian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
