| Message ID | a6c019fd6e47ba0c98fd03e9a4723df0075ad269.1716641976.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | Squash to "use helpers in lib.sh" v5 | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 40 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf__only_bpftest_all_ | success | Success! ✅ |
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
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
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[@]}") }