On Fri, Aug 20, 2021 at 1:43 PM Michael Santana <[email protected]> wrote: > > On Fri, Aug 20, 2021 at 1:09 AM Frode Nordahl > <[email protected]> wrote: > > > > 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. > Thank you for catching this. We hit a snag last week where our CI got > stuck on checkpatch.py for a couple of days without anyone noticing. I > didn't look into it but I hope this is the cause and this patch fixes > it.
You're very welcome, and apologies if I unintentionally caused any issues for the CI. > > > > Also fix the docstring for the if_and_for_end_with_bracket_check > > function. > > > > 0: > > https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ > > Signed-off-by: Frode Nordahl <[email protected]> > > --- > > utilities/checkpatch.py | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > > index 9e8d17653..7c316267f 100755 > > --- a/utilities/checkpatch.py > > +++ b/utilities/checkpatch.py > > @@ -34,6 +34,7 @@ colors = False > > spellcheck_comments = False > > quiet = False > > spell_check_dict = None > > +MAX_LINE_LEN = 79 > > > > > > def open_spell_check_dict(): > > @@ -247,7 +248,7 @@ def if_and_for_whitespace_checks(line): > > > > > > def if_and_for_end_with_bracket_check(line): > > - """Return TRUE if there is not a bracket at the end of an if, for, > > while > > + """Return FALSE if there is not a bracket at the end of an if, for, > > while > Nit-picking: double negatives can be confusing > s/FALSE/TRUE/ > s/not // Ah, thank you for clearing that up, I did stare at that docstring for a while trying to figure out what was meant. > > block which fits on a single line ie: 'if (foo)'""" > > > > def balanced_parens(line): > > @@ -264,6 +265,11 @@ def if_and_for_end_with_bracket_check(line): > > if not balanced_parens(line): > > return True > > > > + # Early return to avoid potential catastrophic backtracking in the > > + # __regex_if_macros regex > > + if len(line) == MAX_LINE_LEN - 1 and line[-1] == ')': > > + return True > > + > Why not >= ? The premise of the check is that if you have room for a bracket on a line with a statement, it should be there. From that I gather that if there is no room you're allowed to put the bracket on the next line, which is the case when you use exactly MAX_LINE_LEN -1 characters for indentation + statement. The catastrophic backtrack appears to occur exactly in this situation when it's not a macro. If the length of the line is >= MAX_LINE_LEN it should be flagged as an error by the line_length_check function. -- Frode Nordahl > > if __regex_ends_with_bracket.search(line) is None and \ > > __regex_if_macros.match(line) is None: > > return False > > @@ -282,7 +288,7 @@ def pointer_whitespace_check(line): > > > > def line_length_check(line): > > """Return TRUE if the line length is too long""" > > - if len(line) > 79: > > + if len(line) > MAX_LINE_LEN: > > print_warning("Line is %d characters long (recommended limit is > > 79)" > > % len(line)) > > return True > > -- > > 2.32.0 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
