[ovs-dev] [PATCH ovn] checkpatch: Avoid a case of catastrophic backtracking
Aaron Conole
aconole at redhat.com
Thu Sep 2 14:04:19 UTC 2021
Hi Frode (et. al.),
Frode Nordahl writes:
> The checkpatch script will hang forever on line 282 of a otherwise
> valid patch [0]. While the root of the issue is probably
> somewhere between the regex itself and the Python re
> implementation, this patch provides an early return to avoid this
> specific issue.
>
> Also fix the docstring for the if_and_for_end_with_bracket_check
> function.
>
> 0: https://patchwork.ozlabs.org/project/ovn/patch/20210819110857.2229769-8-frode.nordahl@canonical.com/
> Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
> ---
Sorry, I was on PTO when this patch came in and missed it. I don't
agree with your proposed fix. It's a bit too conditional (and I was
able to build a test-case that breaks it anyway).
I see it's already been applied to OVN, but I plan to propose the
following for OVS repository (and suggest a similar change to OVN).
The change should be safe because we already validate that we are only
in a for/if/while block, so the early bail to True should be okay.
WDYT?
---
tests/checkpatch.at | 16 ++++++++++++++++
utilities/checkpatch.py | 10 +++++++---
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/tests/checkpatch.at b/tests/checkpatch.at
index 0718acd995..f382149163 100755
--- a/tests/checkpatch.at
+++ b/tests/checkpatch.at
@@ -232,6 +232,22 @@ done
AT_CLEANUP
+AT_SETUP([checkpatch - catastrophic backtracking])
+dnl Special case this rather than using the above construct because sometimes a
+dnl warning needs to be generated for line lengths (f.e. when the 'while'
+dnl keyword is used).
+try_checkpatch \
+ "COMMON_PATCH_HEADER
+ + if (!b_ctx_in->chassis_rec || !b_ctx_in->br_int || !b_ctx_in->ovs_idl_txn)
+ " \
+ "ERROR: Inappropriate bracing around statement
+ #8 FILE: A.c:1:
+ if (!b_ctx_in->chassis_rec || !b_ctx_in->br_int || !b_ctx_in->ovs_idl_txn)
+"
+
+AT_CLEANUP
+
+
AT_SETUP([checkpatch - parenthesized constructs - for])
try_checkpatch \
"COMMON_PATCH_HEADER
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 699fb4b027..16f46c78e8 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -274,9 +274,13 @@ def if_and_for_end_with_bracket_check(line):
if not balanced_parens(line):
return True
- if __regex_ends_with_bracket.search(line) is None and \
- __regex_if_macros.match(line) is None:
- return False
+ if __regex_ends_with_bracket.search(line) is None:
+ if line.endswith("\\") and \
+ __regex_if_macros.match(line) is not None:
+ return True
+ else:
+ return False
+
if __regex_conditional_else_bracing.match(line) is not None:
return False
if __regex_conditional_else_bracing2.match(line) is not None:
--
2.31.1
More information about the dev
mailing list