Re: [LTP] [PATCH] Fix some bashisms

[Subrata Modak <subrata@linux.vnet.ibm.com>]

New Changes to Mikeś comments ?

Regards--
Subrata

On Tue, 2009-06-23 at 17:21 -0400, Mike Frysinger wrote: 
> On Sunday 31 May 2009 17:27:51 Jiri Palecek > wrote:
> > -echo -e "Completed running Default LTP\n\n"
> > +echo "Completed running Default LTP"
> > +echo
> > +echo
> 
> use printf, not bloat the file with echos
> 
> > -echo "Distro type is: $tvar \n"
> > +echo "Distro type is: $tvar"
> >
> 
> these are not equivalent
> 
> > --- a/testcases/commands/unzip/unzip_tests.sh
> > +++ b/testcases/commands/unzip/unzip_tests.sh
> > @@ -62,7 +62,6 @@ chk_ifexists()
> >  # 				- non-zero on failure.
> >  cleanup()
> >  {
> > -	popd
> >  	# remove all the temporary files created by this test.
> >  	tst_resm TINFO "CLEAN: removing \"$LTPTMP\""
> >  	rm -fr "$LTPTMP"
> > @@ -90,7 +89,6 @@ init()
> >  	# create the temporary directory used by this testcase
> >  	LTPTMP=`mktemp -d -t $$.XXXXXX` || tst_resm TBROK "Unable to create
> > temporary directory with: mktemp -d $$.XXXXXX" trap "cleanup" 0
> > -	pushd "$LTPTMP"
> >
> >  	# check if commands tst_*, unzip, awk, etc exists.
> >  	chk_ifexists INIT tst_resm  || return $RC
> 
> umm, you remove the push/popd, but dont replace it with proper cd statements ?  
> i'm guessing you didnt test this change.
> 
> > --- a/testcases/kernel/containers/netns/delchild.sh
> > +++ b/testcases/kernel/containers/netns/delchild.sh
> > -    kill -9 $sshpid $pid > /dev/null 2>&1
> > +    kill -s KILL $sshpid $pid > /dev/null 2>&1
> 
> this isnt a bashism, this is a crappy kill implementation
> 
> >  for tttype in `ls /dev/tty*`
> >  do
> > -device_no=${tttype:8}
> > +device_no=${tttype##/dev/tty}
> > ...
> >  for tttype in `ls /dev/tty*`
> >  do
> > -device_no=${tttype:8}
> > +device_no=${tttype##/dev/tty}
> 
> using the greedy expansion (##) doesnt make much sense.  really should be #.
> 
> > -    let step_errors="$step_errors + 1"
> > +    step_errors=$(($step_errors + 1))
> > -        let i="$i + 1"
> > +        i=$(($i + 1))
> > -            let k="$k + 1"
> > +            k=$(($k + 1))
> > -        let j="$j + 1"
> > +        j=$(($j + 1))
> > .....
> 
> this syntax sucks.  please use something like:
> : $(( step_errors += 1))
> 
> > -    echo $"fs_inod on $TCtmp finished."
> > +    echo "fs_inod on $TCtmp finished."
> 
> these are not equivalent, but i doubt this shell script is using the bash 
> gettext function in the first place, so it's fine
> -mike
> ------------------------------------------------------------------------------
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list