[virt-tools-list] [virt-what PATCH] Remove bashisms

[Guido Günther]

Hi,
On Fri, Aug 04, 2017 at 09:37:53AM -0500, Eric Blake wrote:
> On 08/04/2017 09:25 AM, Guido Günther wrote:
> > Use [ instead of [[ 
> 
> Good.
> 
> > and -a instead of && 
> 
> Absolutely not. Per POSIX, 'test cond1 -a cond2' is not portable; only
> 'test cond1 && test cond2' is.

Other parts of the script use '-a' so I went for that since…

> 
> > so we fall back to test if
> > necessary:
> > 
> >      http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

…states it's allowed (though "strictly conforming" applications shall not
use it. Changed in v2.

> > ---
> >  virt-what.in | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/virt-what.in b/virt-what.in
> > index 8c27b11..56af1ad 100644
> > --- a/virt-what.in
> > +++ b/virt-what.in
> > @@ -360,20 +360,20 @@ if [ "$cpuid" = "OpenBSDVMM58" ]; then
> >  fi
> >  
> >  # Check for LDoms
> > -if [[ "$arch" == sparc* && -e ${root}/dev/mdesc ]]; then
> > +if [ ${arch#sparc} != "$arch" -a -e ${root}/dev/mdesc ]; then
> 
> Incorrect quoting and invalid use of -a.  (Use of [[ doesn't need
> quotes, because it's shell syntax; but use of [ needs quoting in case
> $arch or $root contains spaces or globbing characters)
> 
> Should be:
> 
> if [ "${arch#sparc}" != "$arch ] && [ -e "${root}/dev/mdesc" ]; then
> 
> >      echo ldoms
> > -    if [[ -d ${root}/sys/class/vlds/ctrl && \
> > -             -d ${root}/sys/class/vlds/sp ]]; then
> > +    if [ -d ${root}/sys/class/vlds/ctrl -a \
> > +             -d ${root}/sys/class/vlds/sp ]; then
> 
> Similar incorrect quoting and -a.
> 
> >          echo ldoms-control
> >      else
> >          echo ldoms-guest
> >      fi
> >      MDPROP="${root}/usr/lib/ldoms/mdprop.py"
> > -    if [[ -x ${MDPROP} ]]; then
> > -        if [[ -n $($MDPROP -v iodevice device-type=pciex) ]]; then
> > +    if [ -x ${MDPROP} ]; then
> > +        if [ -n $($MDPROP -v iodevice device-type=pciex) ]; then
> 
> More missing quoting.
> 
> >              echo ldoms-root
> >              echo ldoms-io
> > -        elif [[ -n $($MDPROP -v iov-device vf-id=0) ]]; then
> > +        elif [ -n $($MDPROP -v iov-device vf-id=0) ]; then
> 
> and again

>From what I read in the script these can't be empty or unset but better safe than
sorry. Thanks a lot for the review!
Cheers,
 -- Guido

> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>