Hi Cyril, On 10/02/2014 04:30 PM, Cyril Hrubis wrote: > Hi! >> +# Function: init >> # >> -# Description: - Check if command ip is available. >> +# Description: - Check if command ip is available. >> # - Check if command ifconfig is available. >> # - check if command awk is available. >> # - alias eth0 to eth0:1 with IP 10.1.1.12 >> # >> -# Return - zero on success >> +# Return - zero on success >> # - non zero on failure. return value from commands ($RC) > I would be more aggressiove on cleanup these comments. If nothing else > they are incorrect after the patch is applied as the functions does not > return anything anymore. > > I would remove all useless and redundant information from there, for > this function the only important information is: > > # Alias eth0 to eth0:1 with IP 10.1.1.12 > > But that seems to be covered by the tst_resm TINFO in the middle of the > init() function. > > For the test functions I would keep just short description of what the > test does and remove everything else, the less comments we have the less > incorrect comments we end up with when code is changed.
Absolutely agree. >> - tst_brk TBROK $LTPTMP/tst_ip.out NULL \ >> - "Test #2: modprobe failed to load dummy.o" >> - return $RC >> + modprobe dummy > /dev/null 2>&1 > I'm not sure if we should silence commands that does not produce output > if everything goes right. Because when this starts failing you would > have to remove the redirections to actually figure out what exactly went > wrong. Yes, you are right, I went too far with redirections in this patch (by just replacing previous tmp files with /dev/null). Thanks, Alexey ------------------------------------------------------------------------------ Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list
