svn commit: r217229 - head/usr.sbin/pc-sysinstall/backend

[Pawel Jakub Dawidek]

On Mon, Jan 10, 2011 at 03:52:12PM -0800, Garrett Cooper wrote:
> >> +        if [ "${FS}" != "UFS" -a "${FS}" != "UFS+S" -a "${FS}" != "UFS+J" -a "${FS}" != "UFS+SUJ" ] ; then
> >
> > Something like this should work too:
> >
> >        if [ "${FS%+*}" != "UFS" ]; then
> 
> Except they're catching less than that:
> 
> $ FS=UFS+FOO
> $ echo ${FS%+*}
> UFS
> $

You mean that invalid ${FS} values are catched? The code as it is don't
handle them too. I expect those are handled somewhere earlier. From my
understanding the code wants to dected if this is any configuration of
UFS, so in my opinion my version is better as there are no modifications
needed if some other UFS variant will appear in the future.

> >> +  if [ "$?" != "0" ] ; then return ; fi
> >
> > [ $? -eq 0 ] || return
> 
> if [ $? -eq 0 ]; then
>     return
> fi

In that case -ne, as you reverted the logic.

> is easier to follow for me because more people go buckwild with the
> one-liners (and in some cases have introduced bugs that way because
> they didn't properly think about precedence of the operations, etc).

I kinda started to like very simple and obvious one-liners in sh(1), but
this is just a matter of taste.

> The one-line above if ... fi above is ugly though.

Agreed.

-- 
Pawel Jakub Dawidek                       http://www.wheelsystems.com
pjd at FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-head/attachments/20110111/5f549d23/attachment.pgp