[PATCH] devtools: fix checkpatches subject retrieval from stdin

[PATCH] devtools: fix checkpatches subject retrieval from stdin

[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>
---
 devtools/checkpatches.sh | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

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))
-- 
2.30.2

Re: [PATCH] devtools: fix checkpatches subject retrieval from stdin

[Thomas Monjalon]

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.