Re: [BUG] scripts/checkpatch.pl: false positive missing parentheses

From: Joe Perches
Date: Mon May 07 2018 - 16:43:22 EST


On Mon, 2018-05-07 at 20:31 +0200, Heinrich Schuchardt wrote:
> This patch creates a false positive:
> ERROR: Macros with complex values should be enclosed in parentheses
>
> Here we define a constant that can be used to initialize a structure.
> Adding parentheses would lead to a compile time error:
> error: braced-group within expression allowed only inside a function

>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@xxxxxx>
> ---
> foo.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
> create mode 100644 foo.h
>
> diff --git a/foo.h b/foo.h
> new file mode 100644
> index 000000000000..e2cba533f065
> --- /dev/null
> +++ b/foo.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: BSD-2 */
> +#define EFI_ST_DISK_IMG { 0x00003368, { \
> + {0x00000d40, "\x6f\x63\x00\x2f\x2a\x00\x20\x2a"}, /* oc./*. * */ \
> + {0x00000d48, "\x00\x20\x2a\x2f\x0a\x00\x09\x7b"}, /* . */...{ */ \

I think this line would not do what you expect as the "/* . */" is
a complete comment and the "...{ */" is parsed as code.

> + {0x00000d50, "\x30\x78\x25\x30\x38\x7a\x78\x2c"}, /* 0x%08zx, */ \
> + {0x00000d58, "\x20\x22\x00\x5c\x78\x25\x30\x32"}, /* ".\x%02 */ \
> + {0x00000d60, "\x78\x00\x20\x2a\x2f\x20\x5c\x00"}, /* x. */ \. */ \

here too...

> + {0, NULL} } }

checkpatch will always be a stupid style checker.

Some false positives are expected.

I think this is one of them, but perhaps the bit of
checkpatch logic in the COMPLEX_MACRO test could be
improved by something like the below.

Andy? What do you think?

---
scripts/checkpatch.pl | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 24618dffc5cb..9d3bdab03225 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -785,6 +785,8 @@ our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*};
# Any use must be runtime checked with $^V

our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/;
+our $balanced_braces = qr/(\{(?:[^\{\}]++|(?-1))*\})/;
+our $balanced_brackets = qr/(\[(?:[^\[\]]++|(?-1))*\])/;
our $LvalOrFunc = qr{((?:[\&\*]\s*)?$Lval)\s*($balanced_parens{0,1})\s*};
our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)};

@@ -4953,6 +4955,16 @@ sub process {
$dstat =~ s/^\s*//s;
$dstat =~ s/\s*$//s;

+ # Flatten any parentheses and braces using the
+ # fancy balanced_<foo> tests (perl v5.10+ only)
+ if ($^V && $^V ge 5.10.0) {
+ while ($dstat =~ s/$balanced_parens/1/ ||
+ $dstat =~ s/$balanced_braces/1/ ||
+ $dstat =~ s/$balanced_brackets/1/)
+ {
+ }
+ }
+
# Flatten any parentheses and braces
while ($dstat =~ s/\([^\(\)]*\)/1/ ||
$dstat =~ s/\{[^\{\}]*\}/1/ ||