2011-09-22 11:28:36 +0200, Lukas Czerner:
[...]
> > > + if [ $? -ne 0 ]; then
> > > + error "FAILED. Exitting!"
> > > + fi
> >
> > "$@" || error ...
>
> I guess it is just a matter of taste.
>
> >
> > or
> >
> > if ! "$@"; then
> > error ...
> > fi
>
> That is really not very readable.
[...]
That is how shell works. The syntax is
if
list-of-commands
then
other-list-of-commands
else
yet-another-list-of-commands
fi
I find [ "$?" -ne 0 ] above very confusion and illegible myself.
Why would you run another command that fails if the last one
succeeded?
> > > +}
> > > +
> > > +is_natural() {
> > > + test "$1" -ge 0 &> /dev/null && return 1
> >
> >
> > &> is a bashism.
>
> Probably (#!/bin/bash) :)
But it is called "fsadm.sh", that's misleading.
[...]
> > Again, I think that should be either:
> >
> > set --
> > [ -n "$YES" ] &&
> > set -- -F
> > dry "mkfs.$fstyp" "$@" -b "$(($bsize*1024))" -E stride="${stride},stripe-width=${stripewidth}" "$device"
> >
> > or:
> >
> > force=
> > [ -n "$YES" ] &&
> > force=-F
> >
> > eval 'dry "mkfs.$fstyp" '"$force"' -b "$(($bsize*1024))" -E stride="${stride},stripe-width=${stripewidth}" "$device"'
>
> Ok, I do understand the that I should rather use 'eval', but I do not
> understand why you're trying to get rid of the 'if', it is a bit longer,
> so what ? But is is also more obvious.
"if" is fine, it was just to save me some typing.
> >
> > [...]
> > > + is_natural $NEWSIZE
> > > + [ $? -ne 1 ] && error "$NEWSIZE is not valid number for file system size"
> >
> > With a fixed is_natural,
> >
> > is_natural "$NEWSIZE" || error ...
> >
> > [...]
> > > + for i in $@; do
> >
> > for i do
>
> I am not sure what is wrong with that, it is more obvious and it works
> just fine.
for i in "$@"; do
would have been fine. Again, I can't think of any circumstance
where leaving $@ unquoted would make sense.
--
Stephane
_______________________________________________
linux-lvm mailing list
linux-lvm@redhat.com
https://www.redhat.com/mailman/listinfo/linux-lvm
read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
