diff mbox series

[RFC,net-next,04/10] selftests: forwarding: Simplify forwarding.config import logic

Message ID 20231222135836.992841-5-bpoirier@nvidia.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series selftests: Add TEST_INCLUDES directive and adjust tests to use it | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 5 maintainers not CCed: edumazet@google.com razor@blackwall.org pabeni@redhat.com kuba@kernel.org linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success net selftest script(s) already in Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 19 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Benjamin Poirier Dec. 22, 2023, 1:58 p.m. UTC
The first condition removed by this patch reimplements functionality that
is part of `dirname`:
$ dirname ""
.

Use the libdir variable introduced in the previous patch to import
forwarding.config without duplicating functionality.

Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 tools/testing/selftests/net/forwarding/lib.sh | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

Comments

Vladimir Oltean Dec. 27, 2023, 7:27 p.m. UTC | #1
On Fri, Dec 22, 2023 at 08:58:30AM -0500, Benjamin Poirier wrote:
> The first condition removed by this patch reimplements functionality that
> is part of `dirname`:
> $ dirname ""
> .
> 
> Use the libdir variable introduced in the previous patch to import
> forwarding.config without duplicating functionality.
> 
> Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
> ---
>  tools/testing/selftests/net/forwarding/lib.sh | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index f9e32152f23d..481d9b655a40 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -29,16 +29,12 @@ STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
>  TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
>  TROUTE6=${TROUTE6:=traceroute6}
>  
> -relative_path="${BASH_SOURCE%/*}"
> -if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
> -	relative_path="."
> -fi
> -
> -if [[ -f $relative_path/forwarding.config ]]; then
> -	source "$relative_path/forwarding.config"
> -fi
> -
>  libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
> +
> +if [ -f "$libdir"/forwarding.config ]; then
> +       source "$libdir"/forwarding.config

Nitpick: this used to be indented with tabs, not spaces. Also, any
reason why only "$libdir" is quoted and not the full path, as before?
Otherwise:

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

> +fi
> +
>  source "$libdir"/../lib.sh
>  
>  ##############################################################################
> -- 
> 2.43.0
>
Benjamin Poirier Dec. 28, 2023, 7:07 p.m. UTC | #2
On 2023-12-27 21:27 +0200, Vladimir Oltean wrote:
> On Fri, Dec 22, 2023 at 08:58:30AM -0500, Benjamin Poirier wrote:
> > The first condition removed by this patch reimplements functionality that
> > is part of `dirname`:
> > $ dirname ""
> > .
> > 
> > Use the libdir variable introduced in the previous patch to import
> > forwarding.config without duplicating functionality.
> > 
> > Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
> > ---
> >  tools/testing/selftests/net/forwarding/lib.sh | 14 +++++---------
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> > index f9e32152f23d..481d9b655a40 100644
> > --- a/tools/testing/selftests/net/forwarding/lib.sh
> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
> > @@ -29,16 +29,12 @@ STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
> >  TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
> >  TROUTE6=${TROUTE6:=traceroute6}
> >  
> > -relative_path="${BASH_SOURCE%/*}"
> > -if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
> > -	relative_path="."
> > -fi
> > -
> > -if [[ -f $relative_path/forwarding.config ]]; then
> > -	source "$relative_path/forwarding.config"
> > -fi
> > -
> >  libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
> > +
> > +if [ -f "$libdir"/forwarding.config ]; then
> > +       source "$libdir"/forwarding.config
> 
> Nitpick: this used to be indented with tabs, not spaces.

Thank you for pointing it out. I have fixed it.

> Also, any
> reason why only "$libdir" is quoted and not the full path, as before?

It's not necessary to quote "/forwarding.config" since it doesn't expand
or split, so I did not quote it. Also, the previous code was
inconsistent in its quoting.
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index f9e32152f23d..481d9b655a40 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -29,16 +29,12 @@  STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
 TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
 TROUTE6=${TROUTE6:=traceroute6}
 
-relative_path="${BASH_SOURCE%/*}"
-if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
-	relative_path="."
-fi
-
-if [[ -f $relative_path/forwarding.config ]]; then
-	source "$relative_path/forwarding.config"
-fi
-
 libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
+
+if [ -f "$libdir"/forwarding.config ]; then
+       source "$libdir"/forwarding.config
+fi
+
 source "$libdir"/../lib.sh
 
 ##############################################################################