Messages in this thread |  | | Subject | Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns | | From | Dwaipayan Ray <> | | Date | Sat, 24 Oct 2020 00:44:49 +0530 |
| |
>>> 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.
|  |