Hello! On 1 July 2017 at 01:16, Corentin Labbe <clabbe.montjoie@xxxxxxxxx> wrote: > On Fri, Jun 30, 2017 at 09:50:55PM -0500, Daniel Díaz wrote: >> The ip tool might be provided by another package (such as >> Busybox), not necessarily implementing the -Version switch. >> Just using `type' helps in finding a usable implementation. >> >> Signed-off-by: Daniel Díaz <daniel.diaz@xxxxxxxxxx> >> --- >> tools/testing/selftests/net/netdevice.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/net/netdevice.sh b/tools/testing/selftests/net/netdevice.sh >> index 4e00568..ee5926e 100755 >> --- a/tools/testing/selftests/net/netdevice.sh >> +++ b/tools/testing/selftests/net/netdevice.sh >> @@ -178,7 +178,7 @@ if [ "$(id -u)" -ne 0 ];then >> exit 0 >> fi >> >> -ip -Version 2>/dev/null >/dev/null >> +type ip 2>/dev/null >/dev/null >> if [ $? -ne 0 ];then >> echo "SKIP: Could not run test without the ip tool" >> exit 0 [...] > I prefer to have both way via an "if then else elseif" > It will permit to detect which ip tool is used and handle futur workaround easier. > It will also permit to add more way to detect it. Currently, the if is looking for an implementation of `ip', which is indeed available in my case but fails the test. It works otherwise. As you said a little later, using `type' might not work with all shells (fails on C shells; works on bash, dash, ksh, mksh, zsh), so I'll submit a new version just running the command. Not sure about differentiating the implementation of `ip' at this point, though. Before submitting this patch we discussed internally about how future work might come in later relying on iproute2's behavior, but this might introduce one (or more) variable(s) and logic that won't be used for the time being, sort of pre-optimizing. After all, this patch just improves a check. Greetings! Daniel Díaz daniel.diaz@xxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
