[ovs-dev] checkpatch: Improve accuracy and specificity of sign-off checking.

diff --git a/tests/automake.mk b/tests/automake.mk
index 8224e5a4a22d..01f5077cd6ef 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -24,6 +24,7 @@  COMMON_MACROS_AT = \
 TESTSUITE_AT = \
 	tests/testsuite.at \
 	tests/completion.at \
+	tests/checkpatch.at \
 	tests/library.at \
 	tests/heap.at \
 	tests/bundle.at \
diff --git a/tests/checkpatch.at b/tests/checkpatch.at
new file mode 100644
index 000000000000..ac39d1dfdee5
--- /dev/null
+++ b/tests/checkpatch.at
@@ -0,0 +1,128 @@ 
+AT_BANNER([checkpatch])
+
+OVS_START_SHELL_HELPERS
+# try_checkpatch PATCH [ERRORS]
+#
+# Runs checkpatch under Python 2 and Python 3, if installed, on the given
+# PATCH, expecting the specified set of ERRORS (and warnings).
+try_checkpatch() {
+    AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
+    # Take the patch to test from $1.  Remove an initial four-space indent
+    # from it and, if it is just headers with no body, add a null body.
+    echo "$1" | sed 's/^    //' > test.patch
+    if grep '---' expout >/dev/null 2>&1; then :
+    else
+        printf '\n---\n' >> test.patch
+    fi
+
+    # Take expected output from $2.
+    if test -n "$2"; then
+        echo "$2" | sed 's/^    //' > expout
+    else
+        : > expout
+    fi
+
+    try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
+    try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
+}
+try_checkpatch__() {
+    if test $1 = no; then
+        :
+    elif test -s expout; then
+        AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
+                 [255], [stdout])
+        AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
+    else
+        AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
+    fi
+}
+OVS_END_SHELL_HELPERS
+
+AT_SETUP([checkpatch - sign-offs])
+
+# Sign-off for single author who is also the committer.
+try_checkpatch \
+   "Author: A
+    Committer: A
+
+    Signed-off-by: A"
+try_checkpatch \
+   "Author: A
+    Committer: A" \
+   "ERROR: Author A needs to sign off."
+
+# Sign-off for single author and different committer.
+try_checkpatch \
+   "Author: A
+    Committer: B
+
+    Signed-off-by: A
+    Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+    Committer: B" \
+   "ERROR: Author A needs to sign off.
+    ERROR: Committer B needs to sign off."
+
+# Sign-off for multiple authors with one author also the committer.
+try_checkpatch \
+   "Author: A
+    Committer: A
+
+    Signed-off-by: A
+    Co-authored-by: B
+    Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+    Committer: A
+
+    Co-authored-by: B
+    Signed-off-by: B" \
+   "ERROR: Author A needs to sign off."
+try_checkpatch \
+   "Author: A
+    Committer: A
+
+    Signed-off-by: A
+    Co-authored-by: B" \
+   "ERROR: Co-author B needs to sign off."
+try_checkpatch \
+   "Author: A
+    Committer: A
+
+    Co-authored-by: B" \
+   "ERROR: Author A needs to sign off.
+    ERROR: Co-author B needs to sign off."
+
+# Sign-off for multiple authors and separate committer.
+try_checkpatch \
+   "Author: A
+    Committer: C
+
+    Signed-off-by: A
+    Co-authored-by: B
+    Signed-off-by: B
+    Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+    Committer: C
+
+    Signed-off-by: A
+    Co-authored-by: B
+    Signed-off-by: B" \
+   "ERROR: Committer C needs to sign off."
+
+# Extra sign-offs.
+try_checkpatch \
+   "Author: A
+    Committer: C
+
+    Signed-off-by: A
+    Co-authored-by: B
+    Signed-off-by: B
+    Signed-off-by: C
+    Signed-off-by: D
+    Signed-off-by: E" \
+   "WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: D, E"
+
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 15c385e2cddb..690904e30881 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -21,6 +21,7 @@  m4_include([tests/ovsdb-macros.at])
 m4_include([tests/ofproto-macros.at])
 
 m4_include([tests/completion.at])
+m4_include([tests/checkpatch.at])
 m4_include([tests/bfd.at])
 m4_include([tests/cfm.at])
 m4_include([tests/lacp.at])
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index b2f27f5b7dd1..3c3d2fc7c915 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -668,7 +668,7 @@  def run_file_checks(text):
             check['check'](text)
 
 
-def ovs_checkpatch_parse(text, filename):
+def ovs_checkpatch_parse(text, filename, author=None, committer=None):
     global print_file_name, total_line, checking_file, \
         empty_return_check_state
 
@@ -686,6 +686,8 @@  def ovs_checkpatch_parse(text, filename):
     hunks = re.compile('^(---|\+\+\+) (\S+)')
     hunk_differences = re.compile(
         r'^@@ ([0-9-+]+),([0-9-+]+) ([0-9-+]+),([0-9-+]+) @@')
+    is_author = re.compile(r'^(Author|From): (.*)$', re.I | re.M | re.S)
+    is_committer = re.compile(r'^(Committer: )(.*)$', re.I | re.M | re.S)
     is_signature = re.compile(r'^(Signed-off-by: )(.*)$',
                               re.I | re.M | re.S)
     is_co_author = re.compile(r'^(Co-authored-by: )(.*)$',
@@ -718,13 +720,47 @@  def ovs_checkpatch_parse(text, filename):
             if seppatch.match(line):
                 parse = PARSE_STATE_DIFF_HEADER
                 if not skip_signoff_check:
-                    if len(signatures) == 0:
-                        print_error("No signatures found.")
-                    elif len(signatures) != 1 + len(co_authors):
-                        print_error("Too many signoffs; "
-                                    "are you missing Co-authored-by lines?")
-                    if not set(co_authors) <= set(signatures):
-                        print_error("Co-authored-by/Signed-off-by corruption")
+
+                    # Check that the patch has an author, that the
+                    # author is not among the co-authors, and that the
+                    # co-authors are unique.
+                    if not author:
+                        print_error("Patch lacks author.")
+                        continue
+                    if author in co_authors:
+                        print_error("Author should not be also be co-author.")
+                        continue
+                    if len(set(co_authors)) != len(co_authors):
+                        print_error("Duplicate co-author.")
+
+                    # Check that the author, all co-authors, and the
+                    # committer (if any) signed off.
+                    if author not in signatures:
+                        print_error("Author %s needs to sign off." % author)
+                    for ca in co_authors:
+                        if ca not in signatures:
+                            print_error("Co-author %s needs to sign off." % ca)
+                            break
+                    if author != committer and committer not in signatures:
+                        print_error("Committer %s needs to sign off."
+                                    % committer)
+
+                    # Check for signatures that we do not expect.
+                    # This is only a warning because there can be,
+                    # rarely, a signature chain.
+                    extra_sigs = [x for x in signatures
+                                  if x not in co_authors
+                                  and x != author
+                                  and x != committer]
+                    if extra_sigs:
+                        print_warning("Unexpected sign-offs from developers "
+                                      "who are not authors or co-authors or "
+                                      "committers: %s"
+                                      % ", ".join(extra_sigs))
+            elif is_committer.match(line):
+                committer = is_committer.match(line).group(2)
+            elif is_author.match(line):
+                author = is_author.match(line).group(2)
             elif is_signature.match(line):
                 m = is_signature.match(line)
                 signatures.append(m.group(2))
@@ -815,7 +851,9 @@  def ovs_checkpatch_file(filename):
     for part in mail.walk():
         if part.get_content_maintype() == 'multipart':
             continue
-    result = ovs_checkpatch_parse(part.get_payload(decode=False), filename)
+    result = ovs_checkpatch_parse(part.get_payload(decode=False), filename,
+                                  mail.get('Author', mail['From']),
+                                  mail['Committer'])
     ovs_checkpatch_print_result(result)
     return result