Re: [PATCH 4/5] checkpatch: fix __attribute__ matching

[Andy Whitcroft]

On Mon, Sep 21, 2009 at 07:14:50PM -0700, Daniel Walker wrote:
> In the following code,
> 
> union thread_union init_thread_union
> 	__attribute__((__section__(".data.init_task"))) =
> 		{ INIT_THREAD_INFO(init_task) };
> 
> There is a non-conforming declaration. It should really be like the
> following,
> 
> union thread_union init_thread_union
> 	__attribute__((__section__(".data.init_task"))) = {
> 		INIT_THREAD_INFO(init_task)
> };
> 
> However, checkpatch doesn't catch this right now because it doesn't
> correctly evaluate the "__attribute__". I just fixed it to pattern
> match the attribute in the case above.
> 
> Signed-off-by: Daniel Walker <dwalker@xxxxxxxxxx>
> ---
>  scripts/checkpatch.pl |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index c7f741f..fd4fe03 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -134,7 +134,7 @@ our $Attribute	= qr{
>  			____cacheline_aligned|
>  			____cacheline_aligned_in_smp|
>  			____cacheline_internodealigned_in_smp|
> -			__weak
> +			__weak|(?:__attribute__\(.*\))

The problem with the __attribute__ match is that it is impossible to
sensibly write as a regular-expression as it has nested round brackets
within it.  I do wonder why we care what is before the equals.  I
suspect that any assignment ='s followed by a newline, followed by a {
is wrong.  There are few places that a { is right on the next line.

I'll try that one out and see if it fires any false positives.  Its
passing my tests here.

Could you see if the version at the url below works better for you:

  http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/checkpatch.pl-testing

NOTE: you want at least version 0.29-5-* which is in the process of 
mirroring out.

-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/