diff mbox series

devtools: fix checkpatches subject retrieval from stdin

Message ID 20220929120403.352-1-olivier.matz@6wind.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers show
Series devtools: fix checkpatches subject retrieval from stdin | expand

Checks

Context Check Description
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/checkpatch warning coding style issues

Commit Message

Olivier Matz Sept. 29, 2022, 12:04 p.m. UTC
When passing the patch to checkpatches.sh through stdin, the subject is
retrieved by reading the input until a "Subject:" entry is found. The
rest of the input is piped to checkpatch.pl.

Since the "From:" line is before the "Subject:" line, it won't be sent to
checkpatch.pl, which won't be able to get the author of the commit.
The following error will appear:

> ERROR:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author ''

Do the subject lookup on the temporary file instead of stdin, and
send the whole lines to checkpatch.pl.

The problem is visible since the introduction of this check in linux
checkpatch.pl in version 4.19 (see link below).

Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd2614967d8b
Fixes: 8005feef421d ("scripts: add standard input to checkpatch")
CC: stable@dpdk.org

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 devtools/checkpatches.sh | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

Comments

Thomas Monjalon Nov. 23, 2022, 4:33 p.m. UTC | #1
29/09/2022 14:04, Olivier Matz:
> When passing the patch to checkpatches.sh through stdin, the subject is
> retrieved by reading the input until a "Subject:" entry is found. The
> rest of the input is piped to checkpatch.pl.
> 
> Since the "From:" line is before the "Subject:" line, it won't be sent to
> checkpatch.pl, which won't be able to get the author of the commit.
> The following error will appear:
> 
> > ERROR:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author ''
> 
> Do the subject lookup on the temporary file instead of stdin, and
> send the whole lines to checkpatch.pl.
> 
> The problem is visible since the introduction of this check in linux
> checkpatch.pl in version 4.19 (see link below).
> 
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd2614967d8b
> Fixes: 8005feef421d ("scripts: add standard input to checkpatch")
> CC: stable@dpdk.org
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Thomas Monjalon <thomas@monjalon.net>

Applied, thanks.
diff mbox series

Patch

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 0e82c01947..2b7005dfac 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -278,12 +278,12 @@  print_headline() { # <title>
 total=0
 status=0
 
-check () { # <patch> <commit> <title>
+check () { # <patch-file> <commit>
 	local ret=0
+	local subject=''
 	headline_printed=false
 
 	total=$(($total + 1))
-	! $verbose || print_headline "$3"
 	if [ -n "$1" ] ; then
 		tmpinput=$1
 	else
@@ -298,10 +298,14 @@  check () { # <patch> <commit> <title>
 		fi
 	fi
 
+	# Subject can be on 2 lines
+	subject=$(sed '/^Subject: */!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q' "$tmpinput")
+	! $verbose || print_headline "$subject"
+
 	! $verbose || printf 'Running checkpatch.pl:\n'
 	report=$($DPDK_CHECKPATCH_PATH $options "$tmpinput" 2>/dev/null)
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report" | sed -n '1,/^total:.*lines checked$/p'
 		ret=1
 	fi
@@ -309,7 +313,7 @@  check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking API additions/removals:\n'
 	report=$($VALIDATE_NEW_API "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -317,7 +321,7 @@  check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking forbidden tokens additions:\n'
 	report=$(check_forbidden_additions "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -325,7 +329,7 @@  check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking __rte_experimental tags:\n'
 	report=$(check_experimental_tags "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -333,7 +337,7 @@  check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking __rte_internal tags:\n'
 	report=$(check_internal_tags "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -341,7 +345,7 @@  check () { # <patch> <commit> <title>
 	! $verbose || printf '\nChecking release notes updates:\n'
 	report=$(check_release_notes "$tmpinput")
 	if [ $? -ne 0 ] ; then
-		$headline_printed || print_headline "$3"
+		$headline_printed || print_headline "$subject"
 		printf '%s\n' "$report"
 		ret=1
 	fi
@@ -357,20 +361,10 @@  check () { # <patch> <commit> <title>
 
 if [ -n "$1" ] ; then
 	for patch in "$@" ; do
-		# Subject can be on 2 lines
-		subject=$(sed '/^Subject: */!d;s///;N;s,\n[[:space:]]\+, ,;s,\n.*,,;q' "$patch")
-		check "$patch" '' "$subject"
+		check "$patch" ''
 	done
 elif [ ! -t 0 ] ; then # stdin
-	subject=$(while read header value ; do
-		if [ "$header" = 'Subject:' ] ; then
-			IFS= read next
-			continuation=$(echo "$next" | sed -n 's,^[[:space:]]\+, ,p')
-			echo $value$continuation
-			break
-		fi
-	done)
-	check '' '' "$subject"
+	check '' ''
 else
 	if [ $number -eq 0 ] ; then
 		commits=$(git rev-list --reverse $range)
@@ -378,8 +372,7 @@  else
 		commits=$(git rev-list --reverse --max-count=$number HEAD)
 	fi
 	for commit in $commits ; do
-		subject=$(git log --format='%s' -1 $commit)
-		check '' $commit "$subject"
+		check '' $commit
 	done
 fi
 pass=$(($total - $status))