On Sun, Aug 29, 2021 at 10:56 PM Michael Santana <[email protected]> wrote: > > On Fri, Aug 27, 2021 at 12:18 PM Mark Michelson <[email protected]> wrote: > > > > Hi Frode, > > > > This looks good to me, so > > > > Acked-by: Mark Michelson <[email protected]> > Acked-by: Michael Santana <[email protected]> > > > > I have one small nit below, but I don't think you need to submit a v3 of > > the patch. I think whoever merges this can fix this in the process: > > > > On 8/20/21 10:27 AM, Frode Nordahl 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. > > > > > > 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..a18a26c08 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 TRUE if there is a bracket at the end of an if, for, while > > > 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 > > > + > > > 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)) > > > > This warning should use MAX_LINE_LEN instead of the hard-coded 79.
Thanks for the patch. I addressed Mark's comments and applied the patch to the main branch. Numan > > > > > return True > > > > > > > _______________________________________________ > > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
