Re: [LTP] [PATCH v2 3/3] iptables_tests.sh: Cleanup

[Alexey Kodanev <alexey.kodanev@oracle.com>]

On 01/27/2015 12:16 PM, Zeng Linggang wrote:
> Hi!
> On Mon, 2015-01-26 at 21:37 +0300, Alexey Kodanev wrote:
>> Hi!
>> On 01/21/2015 08:28 AM, Zeng Linggang wrote:
>>> * Use 'test.sh'.
>>> * Use 'tst_*' which defined in 'test.sh'.
>>> * Delete some useless comment.
>>> * Delete 'RC' and 'TFAILCNT'.
>>> * Some cleanup.
>>>
>>> Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
>>> +	modprobe ip_tables > tst_iptables.out 2>&1
>>> +	if [ $? -ne 0 ]; then
>>> +		iptables -L > tst_iptables.out 2>&1
>>> +		if [ $? -ne 0 ]; then
>>> +			tst_brkm TBROK "no iptables support in kenrel."
>>>    		fi
>>>    	fi
>> May be I missed something, but why we need to send output to the file,
>> then, in case of errors, rewrite it with "iptables -L" command?
>>
> The output file is really not necessary here. Maybe it just make our
> screen clearer. I think we need more discussion.
>
>>> +	lsmod | grep "ip_tables" > tst_iptables.out 2>&1
>>> +	if [ $? -eq 0 ]; then
>>> +		iptables -F -t filter > tst_iptables.out 2>&1
>>> +		iptables -F -t nat > tst_iptables.out 2>&1
>>> +		iptables -F -t mangle > tst_iptables.out 2>&1
>>> +		rmmod -v ipt_limit ipt_multiport ipt_LOG ipt_REJECT \
>>> +			 iptable_mangle iptable_nat ip_conntrack \
>>> +			 iptable_filter ip_tables > tst_iptables.out 2>&1
>>>    	fi
>>> -	rm -fr $LTPTMP/tst_iptables.*
>>> -	return $RC
>>> +	tst_rmdir
>> And here, writing output to the file, then removing tmp dir.
>>
> ...
>
>>> +	iptables -A INPUT -s 127.0.0.1 -p icmp -j DROP > tst_iptables.out 2>&1
>>> +	if [ $? -ne 0 ]; then
>>> +		tst_resm TFAIL "iptables command failed to append new rule."
>>> +		return
>>>    	fi
>> At least, we could print 'tst_iptables.out' if the command failed.
>>
> OK. I will do that.
>
>>> +	ping -c 2 127.0.0.1 > tst_iptables.out 2>&1
>>> +	if [ $? -ne 0 ]; then
>>> +		grep "100% packet loss" tst_iptables.out > tst_iptables.err 2>&1
>> The same with tst_iptables.err file.
>>
> OK.
>
>>> +	iptables -F > tst_iptables.out 2>&1
>>> +	if [ $? -ne 0 ]; then
>>> +		tst_resm TFAIL "iptables did not flush all rules."
>>> +		return
>>>    	else
>>> -		tst_resm TINFO "$TCID: iptables logging succsess"
>>> -		tst_resm TPASS "$TCID: iptables can log packets to multiple ports."
>>> +		tst_resm TINFO "iptables logging succsess"
>>> +		tst_resm TPASS "iptables can log packets to multiple ports."
>>>    	fi
>> You don't need "else .. fi" block because you return in the first one.
>>
> I think "else ... fi" is necessary when "$? -eq 0".
> I should remove the 'return' here.

What I mean here is that you could write TINFO/TPASS messages without 
"else":

if [ $? -ne 0 ]; then
     tst_resm TFAIL "error"
     return
fi

tst_resm TPASS "success"
...


Thanks,
Alexey


------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list