Re: [LTP] [PATCH] Fix some bashisms

[Jiri Palecek <jpalecek@web.de>]

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

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