Re: [PATCH mptcp-next 5/5] selftests: mptcp: add mptcp_lib_evts_* helpers

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 19/02/2024 10:29, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> To avoid duplicated code in different MPTCP selftests, we can add and
> use helpers defined in mptcp_lib.sh.
> 
> This patch unifies "pm_nl_ctl events" related code in userspace_pm.sh
> and mptcp_join.sh into four helpers: mptcp_lib_evts_init(), _start(),
> _kill() and _remove(). Define them in mptcp_lib.sh and use these new
> helpers in both scripts.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 62 +++++++-----------
>  .../testing/selftests/net/mptcp/mptcp_lib.sh  | 63 +++++++++++++++++++
>  .../selftests/net/mptcp/userspace_pm.sh       | 31 ++-------
>  3 files changed, 92 insertions(+), 64 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f75d22dbb28d..d4798495a17d 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -33,10 +33,6 @@ ip_mptcp=0
>  check_invert=0
>  validate_checksum=false
>  init=0
> -evts_ns1=""
> -evts_ns2=""
> -evts_ns1_pid=0
> -evts_ns2_pid=0
>  last_test_failed=0
>  last_test_skipped=0
>  last_test_ignored=1
> @@ -148,8 +144,7 @@ init() {
>  	cin=$(mktemp)
>  	cinsent=$(mktemp)
>  	cout=$(mktemp)
> -	evts_ns1=$(mktemp)
> -	evts_ns2=$(mktemp)
> +	mptcp_lib_evts_init
>  
>  	trap cleanup EXIT
>  
> @@ -162,7 +157,7 @@ cleanup()
>  	rm -f "$cin" "$cout" "$sinfail"
>  	rm -f "$sin" "$sout" "$cinsent" "$cinfail"
>  	rm -f "$tmpfile"
> -	rm -rf $evts_ns1 $evts_ns2
> +	mptcp_lib_evts_remove
>  	mptcp_lib_cleanup
>  	cleanup_partial
>  }

Same here, I think it is best to keep these global variables declared
here and pass their names to mptcp_lib_(...) functions.

By doing that, you can keep a different name in mptcp_join.sh and
userspace_pm.sh, and reduce the modifications here below.

(...)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.