[LTP] [PATCH] net/host: update to new api

[LTP] [PATCH] net/host: update to new api

[Kory Maincent]

Fixed awk field separator parsing.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 testcases/network/tcp_cmds/host/host01.sh | 80 +++++------------------
 1 file changed, 18 insertions(+), 62 deletions(-)

diff --git a/testcases/network/tcp_cmds/host/host01.sh b/testcases/network/tcp_cmds/host/host01.sh
index 1308870f7..4987cb838 100755
--- a/testcases/network/tcp_cmds/host/host01.sh
+++ b/testcases/network/tcp_cmds/host/host01.sh
@@ -1,87 +1,43 @@
 #!/bin/sh
 #
-#   Copyright (c) International Business Machines  Corp., 2000
-#
-#   This program is free software;  you can redistribute it and/or modify
-#   it under the terms of the GNU General Public License as published by
-#   the Free Software Foundation; either version 2 of the License, or
-#   (at your option) any later version.
-#
-#   This program is distributed in the hope that it will be useful,
-#   but WITHOUT ANY WARRANTY;  without even the implied warranty of
-#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
-#   the GNU General Public License for more details.
-#
-#   You should have received a copy of the GNU General Public License
-#   along with this program;  if not, write to the Free Software
-#   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-#
-#
-#
-#  FILE   : host
-#
-#  PURPOSE: To test the basic functionality of the `host` command.
-#
-#  SETUP: If "RHOST" is not exported, then the local hostname is used.
-#
-#  HISTORY:
-#    06/06/03 Manoj Iyer manjo@mail.utexas.edu
-#      - Modified to use LTP tests APIs
-#    03/01 Robbie Williamson (robbiew@us.ibm.com)
-#      -Ported
-#
-#
-#-----------------------------------------------------------------------
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) K?ry Maincent <kory.maincent@bootlin.com> 2020
+# Copyright (c) Manoj Iyer <manjo@mail.utexas.edu> 2003
+# Copyright (c) Robbie Williamson <robbiew@us.ibm.com> 2001
+# Copyright (c) International Business Machines  Corp., 2000
+
+TST_SETUP="do_setup"
+TST_TESTFUNC="do_test"
+TST_NEEDS_CMDS="awk host hostname"
+
+. tst_net.sh
 
 do_setup()
 {
     NUMLOOPS=${NUMLOOPS:-1}
     SLEEPTIME=${SLEEPTIME:-0}
-
-    tst_setup
-
-    exists awk host hostname
-
     RHOST=${RHOST:-`hostname`}
-
 }
 
-#-----------------------------------------------------------------------
-#
-# FUNCTION:  do_test
-#
-#-----------------------------------------------------------------------
-
 do_test()
 {
 
-    tst_resm TINFO "test basic functionality of the \`$TC' command."
+    tst_res TINFO "test basic functionality of the host command."
 
-    while [ $TST_COUNT -lt $NUMLOOPS ]; do
+    while [ $TST_COUNT -le $NUMLOOPS ]; do
 
         if rhost_addr=$(host $RHOST); then
-            rhost_addr=$(echo "$rhost_addr" | awk -F, '{print $NF}') >/dev/null 2>&1
-            if ! host $rhost_addr >/dev/null 2>&1; then
-                end_testcase "reverse lookup with host failed"
-            fi
-
+            rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') >/dev/null 2>&1
+            EXPECT_PASS host $rhost_addr \>/dev/null 2>&1
         else
-            end_testcase "host $RHOST on local machine failed"
+            tst_brk TFAIL "host $RHOST on local machine failed"
         fi
 
-        incr_tst_count
+        TST_COUNT=$((TST_COUNT + 1))
         sleep $SLEEPTIME
 
     done
 
 }
 
-#-----------------------------------------------------------------------
-# FUNCTION: MAIN
-#-----------------------------------------------------------------------
-. net_cmdlib.sh
-
-read_opts $*
-do_setup
-do_test
-end_testcase
+tst_run
-- 
2.17.1

[LTP] [PATCH] net/host: update to new api

[Petr Vorel]

Hi Kory,

thanks for your patch.
...
>  do_test()
>  {

> -    tst_resm TINFO "test basic functionality of the \`$TC' command."
> +    tst_res TINFO "test basic functionality of the host command."

> -    while [ $TST_COUNT -lt $NUMLOOPS ]; do
> +    while [ $TST_COUNT -le $NUMLOOPS ]; do
IMHO there is no need to have loop like this.
If required, we'd just add -iN parameter to it in the runtest file (where N is
<1,max int), but IMHO it's enough to test host only once.

>          if rhost_addr=$(host $RHOST); then
> -            rhost_addr=$(echo "$rhost_addr" | awk -F, '{print $NF}') >/dev/null 2>&1
> -            if ! host $rhost_addr >/dev/null 2>&1; then
> -                end_testcase "reverse lookup with host failed"
> -            fi
> -
> +            rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') >/dev/null 2>&1
> +            EXPECT_PASS host $rhost_addr \>/dev/null 2>&1
We need to redirect also second > and &:
EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1

>          else
> -            end_testcase "host $RHOST on local machine failed"
> +            tst_brk TFAIL "host $RHOST on local machine failed"
>          fi

> -        incr_tst_count
> +        TST_COUNT=$((TST_COUNT + 1))
>          sleep $SLEEPTIME
Also sleep time is not really needed.

...

Can I merge this simplified variant?

Kind regards,
Petr

#!/bin/sh
# SPDX-License-Identifier: GPL-2.0-or-later
# Copyright (c) K?ry Maincent <kory.maincent@bootlin.com> 2020
# Copyright (c) Manoj Iyer <manjo@mail.utexas.edu> 2003
# Copyright (c) Robbie Williamson <robbiew@us.ibm.com> 2001
# Copyright (c) International Business Machines  Corp., 2000

TST_TESTFUNC="do_test"
TST_NEEDS_CMDS="awk host hostname"

. tst_net.sh

do_test()
{
	local rhost=${RHOST:-$(hostname)}
	local rhost_addr

    tst_res TINFO "test basic functionality of the host command"

	if rhost_addr=$(host $rhost); then
		rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}')
		EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1
	else
		tst_brk TFAIL "host $rhost on local machine failed"
	fi
}

tst_run

[LTP] [PATCH] net/host: update to new api

[Petr Vorel]

Hi Kory, Alexey,

> Hi Kory,

> thanks for your patch.
> ...
> >  do_test()
> >  {

> > -    tst_resm TINFO "test basic functionality of the \`$TC' command."
> > +    tst_res TINFO "test basic functionality of the host command."

> > -    while [ $TST_COUNT -lt $NUMLOOPS ]; do
> > +    while [ $TST_COUNT -le $NUMLOOPS ]; do
> IMHO there is no need to have loop like this.
> If required, we'd just add -iN parameter to it in the runtest file (where N is
> <1,max int), but IMHO it's enough to test host only once.

> >          if rhost_addr=$(host $RHOST); then
> > -            rhost_addr=$(echo "$rhost_addr" | awk -F, '{print $NF}') >/dev/null 2>&1
> > -            if ! host $rhost_addr >/dev/null 2>&1; then
> > -                end_testcase "reverse lookup with host failed"
> > -            fi
> > -
> > +            rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') >/dev/null 2>&1
> > +            EXPECT_PASS host $rhost_addr \>/dev/null 2>&1
> We need to redirect also second > and &:
> EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1

BTW 2>&1 does not make sense in this context (it's a redirection of error
message of EXPECT_PASS function, which would be to stdout.
I also noticed this part of traceroute01.sh is also wrong:
EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2>&1

It should be:
EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2\>\&1

But IMHO it'd be better in both cases to keep stderr not redirected
(don't hide problems). Therefore, unless you're against it, I'll remove
redirection from traceroute01.sh.

Kind regards,
Petr

[LTP] [PATCH] net/host: update to new api

[=?unknown-8bit?q?K=C3=B6ry?= Maincent]

Hello Petr,

On Mon, 16 Nov 2020 19:48:25 +0100
Petr Vorel <pvorel@suse.cz> wrote:

> > > +            EXPECT_PASS host $rhost_addr \>/dev/null 2>&1  
> > We need to redirect also second > and &:
> > EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1  
> 
> BTW 2>&1 does not make sense in this context (it's a redirection of error
> message of EXPECT_PASS function, which would be to stdout.
> I also noticed this part of traceroute01.sh is also wrong:
> EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2>&1
> 
> It should be:
> EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2\>\&1
> 
> But IMHO it'd be better in both cases to keep stderr not redirected
> (don't hide problems). Therefore, unless you're against it, I'll remove
> redirection from traceroute01.sh.

Yes, your right.
I am new to LTP so I still make some obvious mistakes, thank you for taking
time to correct me.
The simplified variant without stderr redirection seems okay to me.

Regards,

K?ry


-- 
K?ry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[LTP] [PATCH] net/host: update to new api

[Alexey Kodanev]

On 16.11.2020 21:41, Petr Vorel wrote:
> Hi Kory,
> 
> thanks for your patch.
> ...
>>  do_test()
>>  {
> 
>> -    tst_resm TINFO "test basic functionality of the \`$TC' command."
>> +    tst_res TINFO "test basic functionality of the host command."
> 
>> -    while [ $TST_COUNT -lt $NUMLOOPS ]; do
>> +    while [ $TST_COUNT -le $NUMLOOPS ]; do
> IMHO there is no need to have loop like this.
> If required, we'd just add -iN parameter to it in the runtest file (where N is
> <1,max int), but IMHO it's enough to test host only once.
> 
>>          if rhost_addr=$(host $RHOST); then
>> -            rhost_addr=$(echo "$rhost_addr" | awk -F, '{print $NF}') >/dev/null 2>&1
>> -            if ! host $rhost_addr >/dev/null 2>&1; then
>> -                end_testcase "reverse lookup with host failed"
>> -            fi
>> -
>> +            rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}') >/dev/null 2>&1
>> +            EXPECT_PASS host $rhost_addr \>/dev/null 2>&1
> We need to redirect also second > and &:
> EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1
> 
>>          else
>> -            end_testcase "host $RHOST on local machine failed"
>> +            tst_brk TFAIL "host $RHOST on local machine failed"
>>          fi
> 
>> -        incr_tst_count
>> +        TST_COUNT=$((TST_COUNT + 1))
>>          sleep $SLEEPTIME
> Also sleep time is not really needed.
> 
> ...
> 
> Can I merge this simplified variant?
...
> 
> . tst_net.sh
> 
> do_test()
> {
> 	local rhost=${RHOST:-$(hostname)}

Hi Petr, Kory

For new API we shouldn't use RHOST. Also the naming is misleading,
it can be a local host name.


> 	local rhost_addr
> 
>     tst_res TINFO "test basic functionality of the host command"
> 
> 	if rhost_addr=$(host $rhost); then
> 		rhost_addr=$(echo "$rhost_addr" | awk '{print $NF}')
> 		EXPECT_PASS host $rhost_addr \>/dev/null 2\>\&1
> 	else
> 		tst_brk TFAIL "host $rhost on local machine failed"
> 	fi
> }
> 
> tst_run
> 

[LTP] [PATCH] net/host: update to new api

[Petr Vorel]

Hi Alexey,

> > do_test()
> > {
> > 	local rhost=${RHOST:-$(hostname)}

> Hi Petr, Kory

> For new API we shouldn't use RHOST. Also the naming is misleading,
> it can be a local host name.

Thanks for catching it. Suppose this version is ok.

Kind regards,
Petr

TST_TESTFUNC="do_test"
TST_NEEDS_CMDS="awk host hostname"

. tst_net.sh

do_test()
{
	local lhost="$(hostname)"
	local addr

	tst_res TINFO "test basic functionality of the host command"

	if addr=$(host $lhost); then
		addr=$(echo "$addr" | awk '{print $NF}')
		EXPECT_PASS host $addr \>/dev/null
	else
		tst_brk TFAIL "host $lhost on local machine failed"
	fi
}

tst_run