Messages in this thread | | | From | Dwaipayan Ray <> | Date | Sun, 25 Oct 2020 23:40:54 +0530 | Subject | Re: [PATCH v5] checkpatch: extend attributes check to handle more patterns |
| |
On Sun, Oct 25, 2020 at 11:29 PM Joe Perches <joe@perches.com> wrote: > > On Sun, 2020-10-25 at 15:45 +0530, Dwaipayan Ray wrote: > > It is generally preferred that the macros from > > include/linux/compiler_attributes.h are used, unless there > > is a reason not to. > > > > checkpatch currently checks __attribute__ for each of > > packed, aligned, section, printf, scanf, and weak. Other > > declarations in compiler_attributes.h are not handled. > > > > Add a generic test to check the presence of such attributes. > > Some attributes require more specific handling and are kept > > separate. > [] > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -6155,50 +6155,95 @@ sub process { > > } > > } > > > > > > -# Check for __attribute__ packed, prefer __packed > > +# Check for compiler attributes > > if ($realfile !~ m@\binclude/uapi/@ && > > - $line =~ /\b__attribute__\s*\(\s*\(.*\bpacked\b/) { > > - WARN("PREFER_PACKED", > > - "__packed is preferred over __attribute__((packed))\n" . $herecurr); > > - } > > + $rawline =~ /\b__attribute__\s*\(\s*($balanced_parens)\s*\)/) { > > Using $rawline would also change comments and that seems wrong. > Any reason to use $rawline instead of $line? > Hi, Yes I used $line initially but changed it because quoted strings were being replaced.
Like: __attribute__((__section__("_ftrace_events")))
$line in this case was: __attribute__((__section__("XXXXXXXXXXXXXX")))
While $rawline was: __attribute__((__section__("_ftrace_events")))
So to avoid this problem I changed to $rawline.
Is there any other alternative available perhaps? Or should I change back to $rawline?
> [] > > > + if (scalar @conv_array > 0 && $conv_possible == 1) { > > + my $replace = join(' ', @conv_array); > > + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", > > + "$replace is preferred over __attribute__(($attr))\n" . $herecurr) && > > + $fix) { > > + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*\Q$attr\E\s*\)\s*\)/$replace/; > > I looks it would be useful to add > $fixed[$fixlinenr] =~ s/\}\Q$replace\E/} $replace/; > so there's a space added between } and any replacements. > Yes I will do that sure.
Thanks, Dwaipayan.
| |