qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for


From: Jeff Cody
Subject: Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
Date: Wed, 19 Mar 2014 10:45:54 -0400
User-agent: Mutt/1.5.21 (2010-09-15)

On Wed, Mar 19, 2014 at 08:28:07AM -0600, Eric Blake wrote:
> On 03/19/2014 08:19 AM, Jeff Cody wrote:
> 
> >>> +    then
> >>> +        _timed_wait_for ${h} "${@: -1}"
> >>
> >> You have done shift before this. Aren't ${*} the remaining strings to wait 
> >> for ?
> >>
> > 
> > I could probably get rid of the 2nd shift, although I would have to
> > adjust the conditional below.  
> > 
> > I do ${@: -1} because I want the very last whole string to be the item
> > to wait for - this is only needed to accommodate pathnames with spaces
> > inside the QMP string.
> 
> ${@: -1} is not portable:
> 
> $ bash -c 'set 1 2 3; echo ${@: -1}'
> 3
> $ dash -c 'set 1 2 3; echo ${@: -1}'
> dash: 1: Bad substitution
> 
> If you want the last argument, you'll have to do something hideous like:
> 
> eval \${$#}
> 
> Short of using eval, there is no portable way to get at the last
> positional argument in dash.
> 

Yes, and there are likely other bash-isms in some of the shell
scripts in qemu-iotests.  Since #!/bin/bash is explicitly specified,
it seems reasonable that bash-isms would be allowed.  If it was
#!/bin/sh specified as the interpreter, then I would understand
remaining constrained to POSIX-only.

But I think in your next message you have a nice POSIX compatible
method of doing it with shifts, and it is probably best to default to
POSIX when practical.  I'll go ahead and change it to the 
'shift $(($# - 1))' method.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]