linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Bulwahn <lukas.bulwahn@gmail.com>
To: Dwaipayan Ray <dwaipayanray1@gmail.com>,
	 Theodoros Chatziioannidis <sdi1600197@di.uoa.gr>
Cc: linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] [PATCH RFC 1/2] checkpatch: fix multi-statement macro checks
Date: Thu, 1 Oct 2020 10:14:29 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.2010011001340.24571@felia> (raw)
In-Reply-To: <20200930210333.166006-1-dwaipayanray1@gmail.com>



On Thu, 1 Oct 2020, Dwaipayan Ray wrote:

> Checkpatch.pl generates errors which are false positive for
> certain multi-statemenent macros.
> 
> The specific case investigated was whitespace separated statement
> terminated by a semicolon. Checkpatch wrongly classifies such
> as a multi-statement macro.
> 
> For example, commit 4649079b9de1 ("tracing: Make ftrace packed
> events have align of 1") when analyzed with checkpatch generates:
> 
> ERROR: Macros with multiple statements should be enclosed in a do -
> while loop
> +#define __field_packed(type, container, item)    type item;
> 
> The error is misleading in this case and should not be produced.
> The solution undertaken is to exclude any macro which doesn't
> have any non-WSP character after the first and only semicolon(;)

We all know what a semicolon is; so, you do not need to put that in 
braces.

> present in it. This shall allow macros of the form "type item;",
> "type item[size];", etc. to not generate the multi-statement macro
> error.
>

I think the core reason this might work is that it is a string without any 
semicolon and then semicolon is at the very end of this expression.

Okay, we need an evaluation if this makes the whole situation better or 
worse...

Theodoros, can you help us and run checkpatch.pl for this one rule on the 
current kernel source code before and after the patch is applied, and let 
us know which differences you observe?

Dwaipayan, can you try to do a similar evaluation on git commits?

> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> ---
>  scripts/checkpatch.pl | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9e65d21456f1..72c4072307ea 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5346,6 +5346,7 @@ sub process {
>  			    $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ &&	# for (...) bar()
>  			    $dstat !~ /^do\s*{/ &&					# do {...
>  			    $dstat !~ /^\(\{/ &&						# ({...
> +			    $dstat !~ /^[^;]*;\s*$/ &&				# space separated statement;

I do not understand this comment "space separated statement". The example 
above was a macro for a type definition. Isn't that what you have in mind?


Give it another iteration, but I think the patch is pretty clear and so 
let us send it out to Joe and lkml with the next patch version.

Lukas

>  			    $ctx !~ /^.\s*#\s*define\s+TRACE_(?:SYSTEM|INCLUDE_FILE|INCLUDE_PATH)\b/)
>  			{
>  				if ($dstat =~ /^\s*if\b/) {
> -- 
> 2.27.0
> 
> 
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

      parent reply	other threads:[~2020-10-01  8:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-30 21:03 [Linux-kernel-mentees] [PATCH RFC 1/2] checkpatch: fix multi-statement macro checks Dwaipayan Ray
2020-09-30 21:03 ` [Linux-kernel-mentees] [PATCH RFC 2/2] " Dwaipayan Ray
2020-10-01  8:22   ` Lukas Bulwahn
2020-10-01 10:24     ` Dwaipayan Ray
2020-10-01  8:14 ` Lukas Bulwahn [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.2010011001340.24571@felia \
    --to=lukas.bulwahn@gmail.com \
    --cc=dwaipayanray1@gmail.com \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=sdi1600197@di.uoa.gr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).