linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [PATCH] checkpatch: fix multi-statement macro checks for while blocks.
@ 2020-10-01 17:19 Dwaipayan Ray
  2020-10-01 17:25 ` Joe Perches
  0 siblings, 1 reply; 2+ messages in thread
From: Dwaipayan Ray @ 2020-10-01 17:19 UTC (permalink / raw)
  To: joe; +Cc: dwaipayanray1, linux-kernel-mentees, linux-kernel

Checkpatch.pl doesn't have a check for excluding while (...) {...}
blocks from MULTISTATEMENT_MACRO_USE_DO_WHILE error.

For example, running checkpatch.pl on the file mm/maccess.c in the
kernel generates the following error:

ERROR: Macros with complex values should be enclosed in parentheses
+#define copy_from_kernel_nofault_loop(dst, src, len, type, err_label)  \
+       while (len >= sizeof(type)) {                                   \
+               __get_kernel_nofault(dst, src, type, err_label);        \
+               dst += sizeof(type);                                    \
+               src += sizeof(type);                                    \
+               len -= sizeof(type);                                    \
+       }

The error is misleading for this case. Enclosing it in parentheses
doesn't make any sense.

Checkpatch already has an exception list for such common macro types.
Added a new exception for while (...) {...} style blocks to the same.

In addition, the brace flatten logic was modified by changing the
substitution characters from "1" to "1u". This was done to ensure that
macros in the form "#define foo(bar) while(bar){bar--;}" were also
correctly procecssed.

Link: https://lore.kernel.org/linux-kernel-mentees/dc985938aa3986702815a0bd68dfca8a03c85447.camel@perches.com/

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 scripts/checkpatch.pl | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9e65d21456f1..31624bbb342e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5299,9 +5299,9 @@ sub process {
 			$dstat =~ s/\s*$//s;
 
 			# Flatten any parentheses and braces
-			while ($dstat =~ s/\([^\(\)]*\)/1/ ||
-			       $dstat =~ s/\{[^\{\}]*\}/1/ ||
-			       $dstat =~ s/.\[[^\[\]]*\]/1/)
+			while ($dstat =~ s/\([^\(\)]*\)/1u/ ||
+			       $dstat =~ s/\{[^\{\}]*\}/1u/ ||
+			       $dstat =~ s/.\[[^\[\]]*\]/1u/)
 			{
 			}
 
@@ -5342,6 +5342,7 @@ sub process {
 			    $dstat !~ /^\.$Ident\s*=/ &&				# .foo =
 			    $dstat !~ /^(?:\#\s*$Ident|\#\s*$Constant)\s*$/ &&		# stringification #foo
 			    $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ &&	# do {...} while (...); // do {...} while (...)
+			    $dstat !~ /^while\s*$Constant\s*$Constant\s*$/ &&		# while (...) {...}
 			    $dstat !~ /^for\s*$Constant$/ &&				# for (...)
 			    $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ &&	# for (...) bar()
 			    $dstat !~ /^do\s*{/ &&					# do {...
-- 
2.27.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: fix multi-statement macro checks for while blocks.
  2020-10-01 17:19 [Linux-kernel-mentees] [PATCH] checkpatch: fix multi-statement macro checks for while blocks Dwaipayan Ray
@ 2020-10-01 17:25 ` Joe Perches
  0 siblings, 0 replies; 2+ messages in thread
From: Joe Perches @ 2020-10-01 17:25 UTC (permalink / raw)
  To: Dwaipayan Ray, Andrew Morton
  Cc: Andy Whitcroft, linux-kernel-mentees, linux-kernel

On Thu, 2020-10-01 at 22:49 +0530, Dwaipayan Ray wrote:
> Checkpatch.pl doesn't have a check for excluding while (...) {...}
> blocks from MULTISTATEMENT_MACRO_USE_DO_WHILE error.
> 
> For example, running checkpatch.pl on the file mm/maccess.c in the
> kernel generates the following error:
> 
> ERROR: Macros with complex values should be enclosed in parentheses
> +#define copy_from_kernel_nofault_loop(dst, src, len, type, err_label)  \
> +       while (len >= sizeof(type)) {                                   \
> +               __get_kernel_nofault(dst, src, type, err_label);        \
> +               dst += sizeof(type);                                    \
> +               src += sizeof(type);                                    \
> +               len -= sizeof(type);                                    \
> +       }
> 
> The error is misleading for this case. Enclosing it in parentheses
> doesn't make any sense.
> 
> Checkpatch already has an exception list for such common macro types.
> Added a new exception for while (...) {...} style blocks to the same.
> 
> In addition, the brace flatten logic was modified by changing the
> substitution characters from "1" to "1u". This was done to ensure that
> macros in the form "#define foo(bar) while(bar){bar--;}" were also
> correctly procecssed.
> 
> Link: https://lore.kernel.org/linux-kernel-mentees/dc985938aa3986702815a0bd68dfca8a03c85447.camel@perches.com/
> 
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>

Thanks.  Andrew, can you pick this up please?

> ---
>  scripts/checkpatch.pl | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9e65d21456f1..31624bbb342e 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5299,9 +5299,9 @@ sub process {
>  			$dstat =~ s/\s*$//s;
>  
>  			# Flatten any parentheses and braces
> -			while ($dstat =~ s/\([^\(\)]*\)/1/ ||
> -			       $dstat =~ s/\{[^\{\}]*\}/1/ ||
> -			       $dstat =~ s/.\[[^\[\]]*\]/1/)
> +			while ($dstat =~ s/\([^\(\)]*\)/1u/ ||
> +			       $dstat =~ s/\{[^\{\}]*\}/1u/ ||
> +			       $dstat =~ s/.\[[^\[\]]*\]/1u/)
>  			{
>  			}
>  
> @@ -5342,6 +5342,7 @@ sub process {
>  			    $dstat !~ /^\.$Ident\s*=/ &&				# .foo =
>  			    $dstat !~ /^(?:\#\s*$Ident|\#\s*$Constant)\s*$/ &&		# stringification #foo
>  			    $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ &&	# do {...} while (...); // do {...} while (...)
> +			    $dstat !~ /^while\s*$Constant\s*$Constant\s*$/ &&		# while (...) {...}
>  			    $dstat !~ /^for\s*$Constant$/ &&				# for (...)
>  			    $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ &&	# for (...) bar()
>  			    $dstat !~ /^do\s*{/ &&					# do {...

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-10-01 18:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 17:19 [Linux-kernel-mentees] [PATCH] checkpatch: fix multi-statement macro checks for while blocks Dwaipayan Ray
2020-10-01 17:25 ` Joe Perches

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).