Re: [LTP] [PATCH] Fix some bashisms

Subrata Modak
2009-06-30T07:44:26Z

On Thu, 2009-06-25 at 14:39 +0530, Subrata Modak wrote:
> New Changes to Mikeś comments ?

Ping.

>
> 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