On Tue, Aug 10, 2010 at 02:11:03PM -0700, John Fischer wrote:
> All,
>
> Hopefully this is the final code review for this CR. I think this addresses
> all the comments from Nico, Tom and Mark. The complete webreview is available
> at:
>
> http://cr.opensolaris.org/~johnfisc/nodename-6974969/
>
> A diff-ed webreview from the last email is available at:
>
> http://cr.opensolaris.org/~johnfisc/nodename-6974969-diffs/
>
> A diff-ed webreview from the original webreview is available at:
>
> http://cr.opensolaris.org/~johnfisc/nodename-6974969-diffs-orig/
- usr/src/cmd/svc/milestone/identity-node:63
63 +if [[ -z "$hostname" -s /etc/nodename ]]; then
Are you missing an && before the -s?
- Shouldn't /etc/nodename be removed even if empty?
Just rm -f /etc/nodename after line 71?
- Watch out! In [[ ]] ksh uses && and || instead of -a and -o. I
should have explained this earlier; sorry.
(The [[ ]] ksh/bash-ism is a very good one because the parsing is
done in the shell, so there's no ambiguity when an operand starts
with -, for example. And there's no need to quote operands that
consist variable interpolations. And there's a few other benefits to
[[ ]]. Overall it's harder to make silly mistakes with this
ksh/bash-ism, but you do have to be aware of these features. There's
nothing like that to know about $(( )), other than the fact that it's
safe to nest that.)
Otherwise it looks good to me.
Nico
--
_______________________________________________
networking-discuss mailing list
networking-discuss@opensolaris.org
