Re: [LTP] [PATCH] Fix some bashisms

Jiri Palecek
2009-07-06T23:10:36Z

On Tuesday 30 June 2009 09:41:10 Subrata Modak wrote:
> On Thu, 2009-06-25 at 14:39 +0530, Subrata Modak wrote:
> > New Changes to Mikeś comments ?
>
> Ping.

Ok, I finally got time to look into this. I'll send a separate patch which reflects the 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:
> > > > --- 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.

Oh yes, I did, but I tested it with other changes which rendered the pushd/popd unnecessary. You're right this wouldn't work.

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

Formally speaking, this is an XSI extension to POSIX. I don't think this "-9" is any better than "-s KILL", so I won't change this.

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

If it hurts your aesthetic sense ... ok.

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