Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns

[Dwaipayan Ray]

> > > And you could check using
> > >
> > >          $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*)/
> > >
> > > and check for all attributes in $1 after
> > > stripping the leading and trailing parens
> > > and any leading and trailing underscores
> > > from each attribute.
> > >
> > > And you only need one hash and you should
> > > check for existence of the key rather than
> > > have multiple lists.
> > >
> > >          if (exists($attributes($attr))) {
> > Okay thanks!
> > But what should be done for the attributes which are
> > parameterized, like __aligned__(x). Should all the __
> > for these entries be trimmed too?
> yes
>
> > There are also
> > cases where there are multiple versions like:
> >
> > __aligned__
> > __aligned__(x)
> $ git grep __aligned__ | grep -v -P '__aligned__\s*\('
>
> AFAIK: There is only one use of bare __aligned__ and
> that's in compiler_attributes.h
>
> > To help differentiate between them what can be done?
> > Should i make the keys as:
> >
> > aligned
> > aligned__(
> >
> > instead of
> >
> > __aligned__
> > __aligned__(
> Just use aligned
>
> Just fyi:
>
> these are the uses of __attribute__ in the kernel
> with all the underscores and spaces removed so there's
> some value in finding the multiple actual attributes .


Hi,
I modified the check to check the attributes from the map.
There are two checks - one for the normal attributes and
one for the ones with arguments, which needs just a bit more processing.

So attributes like __packed__ as well as those like
__aligned__(x) are handled.

What do you think?

---
+            $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*\)/) {
+            my $attr = trim($1);
+            $attr =~ s/\(\s*_*(.*)\)/$1/;
+            while($attr =~ s/(.*)_$/$1/) {}  # Remove trailing underscores
+
+            my %attr_list = (
+                "alias"            => "__alias",
+                "aligned"        => "__aligned",
+                "always_inline"     => "__always_inline",
+                "assume_aligned"    => "__assume_aligned",
+                "cold"            => "__cold",
+                "const"            => "__const",
+                "copy"            => "__copy",
+                "designated_init"        => "__designated_init",
+                "externally_visible"    => "__visible",
+                "fallthrough"        => "fallthrough",
+                "gnu_inline"        => "__gnu_inline",
+                "malloc"        => "__malloc",
+                "mode"            => "__mode",
+                "no_caller_saved_registers" => 
"__no_caller_saved_registers",
+                "noclone"        => "__noclone",
+                "noinline"        => "noinline",
+                "nonstring"        => "__nonstring",
+                "noreturn"        => "__noreturn",
+                "packed"        => "__packed",
+                "pure"            => "__pure",
+                "used"            => "__used"
+            );
+
+            if (exists($attr_list{$attr})) {
+                my $new = $attr_list{$attr};
+                WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+                     "$new is preffered over 
__attribute__((__${attr}__))\n" . $herecurr);
+            }
+
+            # Check for attributes with parameters, like copy__(symbol)
+            if ($attr =~ /(.*)__(\(.*\))/) {
+                if (exists($attr_list{$1})) {
+                    my $new = $attr_list{$1};
+                    WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+                         "$new$2 is preffered over 
__attribute__((__${attr}))\n" . $herecurr);
+                }
+            }
---

If this is okay I would like to send in a proper v3.

Thanks,
Dwaipayan.