From: Geliang Tang <tanggeliang@kylinos.cn>

I think "ns_temp" is a better name than "_ns" here.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/lib.sh | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index b883289ec4a1..97f3d7d6b913 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -167,11 +167,11 @@ cleanup_all_ns()
 # setup_ns local remote
 setup_ns()
 {
-	local _ns=""
+	local ns_temp=""
 	local ns_name=""
 	local ns_list=()
 	for ns_name in "$@"; do
-		if [ "${ns_name}" = "_ns" ]; then
+		if [ "${ns_name}" = "ns_temp" ]; then
 			echo "Failed to setup namespace '${ns_name}': invalid name"
 			cleanup_ns "${ns_list[@]}"
 			exit $ksft_fail
@@ -179,20 +179,20 @@ setup_ns()
 
 		# Some test may setup/remove same netns multi times
 		if [ -z "${!ns_name}" ]; then
-			_ns="${ns_name,,}-$(mktemp -u XXXXXX)"
-			eval "${ns_name}=${_ns}"
+			ns_temp="${ns_name,,}-$(mktemp -u XXXXXX)"
+			eval "${ns_name}=${ns_temp}"
 		else
-			_ns="${!ns_name}"
-			cleanup_ns "${_ns}"
+			ns_temp="${!ns_name}"
+			cleanup_ns "${ns_temp}"
 		fi
 
-		if ! ip netns add "${_ns}"; then
+		if ! ip netns add "${ns_temp}"; then
 			echo "Failed to create namespace $ns_name"
 			cleanup_ns "${ns_list[@]}"
 			return $ksft_skip
 		fi
-		ip -n "${_ns}" link set lo up
-		ns_list+=("${_ns}")
+		ip -n "${ns_temp}" link set lo up
+		ns_list+=("${ns_temp}")
 	done
 	NS_LIST+=("${ns_list[@]}")
 }
-- 
2.43.0

Hi Geliang,

On 25/05/2024 15:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> I think "ns_temp" is a better name than "_ns" here.

Good point. But when looking at that, I think it would be even better to
get rid of this variable.

I can suggest a patch in a v6.

> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/net/lib.sh | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> index b883289ec4a1..97f3d7d6b913 100644
> --- a/tools/testing/selftests/net/lib.sh
> +++ b/tools/testing/selftests/net/lib.sh
> @@ -167,11 +167,11 @@ cleanup_all_ns()
>  # setup_ns local remote
>  setup_ns()
>  {
> -	local _ns=""
> +	local ns_temp=""
>  	local ns_name=""
>  	local ns_list=()
>  	for ns_name in "$@"; do
> -		if [ "${ns_name}" = "_ns" ]; then
> +		if [ "${ns_name}" = "ns_temp" ]; then
>  			echo "Failed to setup namespace '${ns_name}': invalid name"
>  			cleanup_ns "${ns_list[@]}"
>  			exit $ksft_fail

I don't know if this check still makes sense. It could also conflict
with ns_name...

> @@ -179,20 +179,20 @@ setup_ns()
>  
>  		# Some test may setup/remove same netns multi times
>  		if [ -z "${!ns_name}" ]; then
> -			_ns="${ns_name,,}-$(mktemp -u XXXXXX)"
> -			eval "${ns_name}=${_ns}"
> +			ns_temp="${ns_name,,}-$(mktemp -u XXXXXX)"
> +			eval "${ns_name}=${ns_temp}"
>  		else
> -			_ns="${!ns_name}"
> -			cleanup_ns "${_ns}"
> +			ns_temp="${!ns_name}"
> +			cleanup_ns "${ns_temp}"
>  		fi
>  
> -		if ! ip netns add "${_ns}"; then
> +		if ! ip netns add "${ns_temp}"; then
>  			echo "Failed to create namespace $ns_name"
>  			cleanup_ns "${ns_list[@]}"
>  			return $ksft_skip
>  		fi
> -		ip -n "${_ns}" link set lo up
> -		ns_list+=("${_ns}")
> +		ip -n "${ns_temp}" link set lo up
> +		ns_list+=("${ns_temp}")
>  	done
>  	NS_LIST+=("${ns_list[@]}")
>  }

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

On Mon, 2024-05-27 at 12:14 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 25/05/2024 15:01, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > I think "ns_temp" is a better name than "_ns" here.
> 
> Good point. But when looking at that, I think it would be even better
> to
> get rid of this variable.
> 
> I can suggest a patch in a v6.

Sure, let's drop this set and wait for a v6.

Thanks,
-Geliang

> 
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  tools/testing/selftests/net/lib.sh | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/lib.sh
> > b/tools/testing/selftests/net/lib.sh
> > index b883289ec4a1..97f3d7d6b913 100644
> > --- a/tools/testing/selftests/net/lib.sh
> > +++ b/tools/testing/selftests/net/lib.sh
> > @@ -167,11 +167,11 @@ cleanup_all_ns()
> >  # setup_ns local remote
> >  setup_ns()
> >  {
> > -	local _ns=""
> > +	local ns_temp=""
> >  	local ns_name=""
> >  	local ns_list=()
> >  	for ns_name in "$@"; do
> > -		if [ "${ns_name}" = "_ns" ]; then
> > +		if [ "${ns_name}" = "ns_temp" ]; then
> >  			echo "Failed to setup namespace
> > '${ns_name}': invalid name"
> >  			cleanup_ns "${ns_list[@]}"
> >  			exit $ksft_fail
> 
> I don't know if this check still makes sense. It could also conflict
> with ns_name...
> 
> > @@ -179,20 +179,20 @@ setup_ns()
> >  
> >  		# Some test may setup/remove same netns multi
> > times
> >  		if [ -z "${!ns_name}" ]; then
> > -			_ns="${ns_name,,}-$(mktemp -u XXXXXX)"
> > -			eval "${ns_name}=${_ns}"
> > +			ns_temp="${ns_name,,}-$(mktemp -u XXXXXX)"
> > +			eval "${ns_name}=${ns_temp}"
> >  		else
> > -			_ns="${!ns_name}"
> > -			cleanup_ns "${_ns}"
> > +			ns_temp="${!ns_name}"
> > +			cleanup_ns "${ns_temp}"
> >  		fi
> >  
> > -		if ! ip netns add "${_ns}"; then
> > +		if ! ip netns add "${ns_temp}"; then
> >  			echo "Failed to create namespace $ns_name"
> >  			cleanup_ns "${ns_list[@]}"
> >  			return $ksft_skip
> >  		fi
> > -		ip -n "${_ns}" link set lo up
> > -		ns_list+=("${_ns}")
> > +		ip -n "${ns_temp}" link set lo up
> > +		ns_list+=("${ns_temp}")
> >  	done
> >  	NS_LIST+=("${ns_list[@]}")
> >  }
> 
> Cheers,
> Matt