On Mon, Feb 12, 2024 at 1:30 PM Dumitru Ceara <[email protected]> wrote:

> On 2/8/24 19:17, Ales Musil wrote:
> > To avoid issues with hardcoded table numbers in future add rule
> > into check patch. The rule is only warning because there are still
> > legitimate use cases and not everything can be abstracted.
> >
> > Signed-off-by: Ales Musil <[email protected]>
> > ---
>
> Hi Ales,
>

Hi Dumitru,

thank you for the review.


>
> >  utilities/checkpatch.py | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
>
> Would it be possible to add a test in checkpatch.at too?
>

Will do in v4.


>
> > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> > index 52d3fa845..9f7a48c7c 100755
> > --- a/utilities/checkpatch.py
> > +++ b/utilities/checkpatch.py
> > @@ -202,6 +202,7 @@ __regex_if_macros = re.compile(r'^ +(%s)
> \([\S]([\s\S]+[\S])*\) { +\\' %
> >                                 __parenthesized_constructs)
> >  __regex_nonascii_characters = re.compile("[^\u0000-\u007f]")
> >  __regex_efgrep = re.compile(r'.*[ef]grep.*$')
> > +__regex_hardcoded_table =
> re.compile(r'(table=[0-9]+)|(resubmit\(,[0-9]+\))')
>
> This won't match on lines that include "table=XYZ" or "resubmit(,XYZ)"
> but start with something else.
>
> Should it be something like this?
>
> __regex_hardcoded_table =
> re.compile(r'.*(table=[0-9]+)|(resubmit\(,[0-9]+\))')
>

Yeah most likely, I didn't realize that python match is only from the start
even without the "^" specifier.


>
> >
> >  skip_leading_whitespace_check = False
> >  skip_trailing_whitespace_check = False
> > @@ -371,6 +372,10 @@ def has_efgrep(line):
> >      """Returns TRUE if the current line contains 'egrep' or 'fgrep'."""
> >      return __regex_efgrep.match(line) is not None
> >
> > +def has_hardcoded_table(line):
> > +    """Return TRUE if the current line contains table=<NUMBER> or
> > +       resubmit(,<NUMBER>)"""
> > +    return __regex_hardcoded_table.match(line) is not None
> >
> >  def filter_comments(current_line, keep=False):
> >      """remove all of the c-style comments in a line"""
> > @@ -656,6 +661,13 @@ checks = [
> >       'check': lambda x: has_efgrep(x),
> >       'print':
> >       lambda: print_error("grep -E/-F should be used instead of
> egrep/fgrep")},
> > +
> > +    {'regex': r'\.at$', 'match_name': None,
> > +     'check': lambda x: has_hardcoded_table(x),
> > +     'print':
> > +         lambda: print_warning("Use of hardcoded table=<NUMBER> or"
> > +                               " resubmit=(,<NUMBER>) is discouraged in
> tests."
> > +                               " Consider using MACRO instead.")},
> >  ]
> >
> >
>
> Thanks,
> Dumitru
>
>
Thanks.
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to