| Message ID | 20180626125838.3289-1-aconole@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] checkpatch: fix scissors line regex | expand |
On Tue, Jun 26, 2018 at 08:58:38AM -0400, Aaron Conole wrote: > The scissors line is always three dashes on a line. The regex would > previously match on any three dashes in a row. > > This means that a patch (such as [1]) would trigger the parser state > machine to advance beyond the signed-off checks. This bounds the > check only to run on specific lines. > > 1: https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348625.html > > Fixes: c599d5ccf316 ("checkpatch.py: A simple script for finding patch issues") > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- > utilities/checkpatch.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > index 5a6d5f5ff..80b955f6b 100755 > --- a/utilities/checkpatch.py > +++ b/utilities/checkpatch.py > @@ -619,7 +619,7 @@ def ovs_checkpatch_parse(text, filename): > parse = 0 > current_file = filename if checking_file else '' > previous_file = '' > - scissors = re.compile(r'^[\w]*---[\w]*') > + scissors = re.compile(r'^[\w]*---[\w]*$') > hunks = re.compile('^(---|\+\+\+) (\S+)') > hunk_differences = re.compile( > r'^@@ ([0-9-+]+),([0-9-+]+) ([0-9-+]+),([0-9-+]+) @@') I'm not sure that this is what Git calls a scissors line. git-mailinfo says: --scissors Remove everything in body before a scissors line. A line that mainly consists of scissors (either ">8" or "8<") and perforation (dash "-") marks is called a scissors line, and is used to request the reader to cut the message at that line. If such a line appears in the body of the message before the patch, everything before it (including the scissors line itself) is ignored when this option is used. This is useful if you want to begin your message in a discussion thread with comments and suggestions on the message you are responding to, and to conclude it with a patch submission, separating the discussion and the beginning of the proposed commit log message with a scissors line. This can be enabled by default with the configuration option mailinfo.scissors. I personally use the following line for scissors: --8<--------------------------cut here-------------------------->8--
Ben Pfaff <blp@ovn.org> writes: > On Tue, Jun 26, 2018 at 08:58:38AM -0400, Aaron Conole wrote: >> The scissors line is always three dashes on a line. The regex would >> previously match on any three dashes in a row. >> >> This means that a patch (such as [1]) would trigger the parser state >> machine to advance beyond the signed-off checks. This bounds the >> check only to run on specific lines. >> >> 1: https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348625.html >> >> Fixes: c599d5ccf316 ("checkpatch.py: A simple script for finding patch issues") >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> --- >> utilities/checkpatch.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py >> index 5a6d5f5ff..80b955f6b 100755 >> --- a/utilities/checkpatch.py >> +++ b/utilities/checkpatch.py >> @@ -619,7 +619,7 @@ def ovs_checkpatch_parse(text, filename): >> parse = 0 >> current_file = filename if checking_file else '' >> previous_file = '' >> - scissors = re.compile(r'^[\w]*---[\w]*') >> + scissors = re.compile(r'^[\w]*---[\w]*$') >> hunks = re.compile('^(---|\+\+\+) (\S+)') >> hunk_differences = re.compile( >> r'^@@ ([0-9-+]+),([0-9-+]+) ([0-9-+]+),([0-9-+]+) @@') > > I'm not sure that this is what Git calls a scissors line. git-mailinfo > says: Oups. I've always called both a scissors line (whether I use the scissors mark or not), but it seems I really should distinguish scissors from perforation. Regardless, the change is needed to ensure we don't accidentally advance the patch parsing state machine. I'll re-spin it with a better commit message, and likely will have another patch to address the other flag that happened with Darrell's patch.
On Wed, Jun 27, 2018 at 09:17:16AM -0400, Aaron Conole wrote: > Ben Pfaff <blp@ovn.org> writes: > > > On Tue, Jun 26, 2018 at 08:58:38AM -0400, Aaron Conole wrote: > >> The scissors line is always three dashes on a line. The regex would > >> previously match on any three dashes in a row. > >> > >> This means that a patch (such as [1]) would trigger the parser state > >> machine to advance beyond the signed-off checks. This bounds the > >> check only to run on specific lines. > >> > >> 1: https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348625.html > >> > >> Fixes: c599d5ccf316 ("checkpatch.py: A simple script for finding patch issues") > >> Signed-off-by: Aaron Conole <aconole@redhat.com> > >> --- > >> utilities/checkpatch.py | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > >> index 5a6d5f5ff..80b955f6b 100755 > >> --- a/utilities/checkpatch.py > >> +++ b/utilities/checkpatch.py > >> @@ -619,7 +619,7 @@ def ovs_checkpatch_parse(text, filename): > >> parse = 0 > >> current_file = filename if checking_file else '' > >> previous_file = '' > >> - scissors = re.compile(r'^[\w]*---[\w]*') > >> + scissors = re.compile(r'^[\w]*---[\w]*$') > >> hunks = re.compile('^(---|\+\+\+) (\S+)') > >> hunk_differences = re.compile( > >> r'^@@ ([0-9-+]+),([0-9-+]+) ([0-9-+]+),([0-9-+]+) @@') > > > > I'm not sure that this is what Git calls a scissors line. git-mailinfo > > says: > > Oups. I've always called both a scissors line (whether I use the > scissors mark or not), but it seems I really should distinguish scissors > from perforation. > > Regardless, the change is needed to ensure we don't accidentally advance > the patch parsing state machine. > > I'll re-spin it with a better commit message, and likely will have > another patch to address the other flag that happened with Darrell's > patch. Respinning with a better commit message sounds like a good outcome to me. Thanks.
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 5a6d5f5ff..80b955f6b 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -619,7 +619,7 @@ def ovs_checkpatch_parse(text, filename): parse = 0 current_file = filename if checking_file else '' previous_file = '' - scissors = re.compile(r'^[\w]*---[\w]*') + scissors = re.compile(r'^[\w]*---[\w]*$') hunks = re.compile('^(---|\+\+\+) (\S+)') hunk_differences = re.compile( r'^@@ ([0-9-+]+),([0-9-+]+) ([0-9-+]+),([0-9-+]+) @@')
The scissors line is always three dashes on a line. The regex would previously match on any three dashes in a row. This means that a patch (such as [1]) would trigger the parser state machine to advance beyond the signed-off checks. This bounds the check only to run on specific lines. 1: https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348625.html Fixes: c599d5ccf316 ("checkpatch.py: A simple script for finding patch issues") Signed-off-by: Aaron Conole <aconole@redhat.com> --- utilities/checkpatch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)