check_transfer() helper is defined both in mptcp_connect.sh and
mptcp_sockopt.sh, export it into mptcp_lib.sh and rename it with
mptcp_lib_ prefix. Use this new helper in both scripts.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../selftests/net/mptcp/mptcp_connect.sh      | 29 ++-----------------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 24 +++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      | 28 +-----------------
 3 files changed, 27 insertions(+), 54 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index dc4a1dd3566d..0bd2392ae442 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -254,31 +254,6 @@ else
 	set_ethtool_flags "$ns4" ns4eth3 "$ethtool_args"
 fi
 
-print_file_err()
-{
-	ls -l "$1" 1>&2
-	echo "Trailing bytes are: "
-	tail -c 27 "$1"
-}
-
-check_transfer()
-{
-	local in=$1
-	local out=$2
-	local what=$3
-
-	cmp "$in" "$out" > /dev/null 2>&1
-	if [ $? -ne 0 ] ;then
-		echo "[ FAIL ] $what does not match (in, out):"
-		print_file_err "$in"
-		print_file_err "$out"
-
-		return 1
-	fi
-
-	return 0
-}
-
 check_mptcp_disabled()
 {
 	local disabled_ns="ns_disabled-$rndh"
@@ -483,9 +458,9 @@ do_transfer()
 		return 1
 	fi
 
-	check_transfer $sin $cout "file received by client"
+	mptcp_lib_check_transfer $sin $cout "file received by client"
 	retc=$?
-	check_transfer $cin $sout "file received by server"
+	mptcp_lib_check_transfer $cin $sout "file received by server"
 	rets=$?
 
 	local stat_synrx_now_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableSYNRX")
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 7b0d03c40f89..fba62cdef2cd 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -353,3 +353,27 @@ mptcp_lib_make_file() {
 	dd if=/dev/urandom of="$name" bs=$bs count=$size 2> /dev/null
 	echo -e "\nMPTCP_TEST_FILE_END_MARKER" >> "$name"
 }
+
+print_file_err()
+{
+	ls -l "$1" 1>&2
+	echo "Trailing bytes are: "
+	tail -c 27 "$1"
+}
+
+mptcp_lib_check_transfer() {
+	local in=$1
+	local out=$2
+	local what=$3
+
+	cmp "$in" "$out" > /dev/null 2>&1
+	if [ $? -ne 0 ] ;then
+		echo "[ FAIL ] $what does not match (in, out):"
+		print_file_err "$in"
+		print_file_err "$out"
+
+		return 1
+	fi
+
+	return 0
+}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 39128fca99dd..aa4b9a4e6a56 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -135,32 +135,6 @@ check_mark()
 	return 0
 }
 
-print_file_err()
-{
-	ls -l "$1" 1>&2
-	echo "Trailing bytes are: "
-	tail -c 27 "$1"
-}
-
-check_transfer()
-{
-	local in=$1
-	local out=$2
-	local what=$3
-
-	cmp "$in" "$out" > /dev/null 2>&1
-	if [ $? -ne 0 ] ;then
-		echo "[ FAIL ] $what does not match (in, out):"
-		print_file_err "$in"
-		print_file_err "$out"
-		ret=1
-
-		return 1
-	fi
-
-	return 0
-}
-
 do_transfer()
 {
 	local listener_ns="$1"
@@ -232,7 +206,7 @@ do_transfer()
 		check_mark $connector_ns 4 || retc=1
 	fi
 
-	check_transfer $cin $sout "file received by server"
+	mptcp_lib_check_transfer $cin $sout "file received by server"
 	rets=$?
 
 	mptcp_lib_result_code "${retc}" "mark ${ip}"
-- 
2.35.3

Hi Geliang,

On 25/09/2023 10:42, Geliang Tang wrote:
> check_transfer() helper is defined both in mptcp_connect.sh and
> mptcp_sockopt.sh, export it into mptcp_lib.sh and rename it with
> mptcp_lib_ prefix. Use this new helper in both scripts.

Good idea!

(...)

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 7b0d03c40f89..fba62cdef2cd 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -353,3 +353,27 @@ mptcp_lib_make_file() {
>  	dd if=/dev/urandom of="$name" bs=$bs count=$size 2> /dev/null
>  	echo -e "\nMPTCP_TEST_FILE_END_MARKER" >> "$name"
>  }
> +
> +print_file_err()

Please prefix it with "mptcp_lib_" + add a comment:

  # $1: file

> +{
> +	ls -l "$1" 1>&2

Please use {} around variables: "${1}"

(like in the rest of the file)

> +	echo "Trailing bytes are: "
> +	tail -c 27 "$1"
> +}
> +
> +mptcp_lib_check_transfer() {

Please add a comment:

  # $1: input file ; $2: output file ; $3: what kind of file

> +	local in=$1

Please use {} and quotes around variables: "${1}". Same below.

> +	local out=$2
> +	local what=$3
> +
> +	cmp "$in" "$out" > /dev/null 2>&1
> +	if [ $? -ne 0 ] ;then

Small detail: probably better to do this (I guess Shellcheck would
complain if it is not done like that)

  if ! cmp "$in" "$out" > /dev/null 2>&1; then

> +		echo "[ FAIL ] $what does not match (in, out):"
> +		print_file_err "$in"
> +		print_file_err "$out"
> +
> +		return 1
> +	fi
> +
> +	return 0
> +}
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index 39128fca99dd..aa4b9a4e6a56 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -135,32 +135,6 @@ check_mark()
>  	return 0
>  }
>  
> -print_file_err()
> -{
> -	ls -l "$1" 1>&2
> -	echo "Trailing bytes are: "
> -	tail -c 27 "$1"
> -}
> -
> -check_transfer()
> -{
> -	local in=$1
> -	local out=$2
> -	local what=$3
> -
> -	cmp "$in" "$out" > /dev/null 2>&1
> -	if [ $? -ne 0 ] ;then
> -		echo "[ FAIL ] $what does not match (in, out):"
> -		print_file_err "$in"
> -		print_file_err "$out"
> -		ret=1

Could you add a comment in the commit message: here it is OK to drop
'ret=1' because it will be set in run_tests() anyway (if I'm not mistaken).

Otherwise a reviewer could think the test is no longer marked as failed
in case of error and he might ask the question or take time to find the
answer and we want to avoid that ;)

> -
> -		return 1
> -	fi
> -
> -	return 0
> -}
> -
>  do_transfer()
>  {
>  	local listener_ns="$1"
> @@ -232,7 +206,7 @@ do_transfer()
>  		check_mark $connector_ns 4 || retc=1
>  	fi
>  
> -	check_transfer $cin $sout "file received by server"
> +	mptcp_lib_check_transfer $cin $sout "file received by server"
>  	rets=$?
>  
>  	mptcp_lib_result_code "${retc}" "mark ${ip}"

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net