On Thu, Aug 05, 2010 at 02:03:28PM -0700, John Fischer wrote:
> The code review is located at:
>
> http://cr.opensolaris.org/~johnfisc/nodename-6974969
- usr/src/cmd/svc/milestone/identity-node:60,63,99
Style: use $() instead of ``.
- usr/src/cmd/svc/milestone/identity-node:61,100
s/==/=/
s/"\\"\\""/'""'/
In other words:
[[ "$hostname" = '""' ]] && hostname=""
- usr/src/cmd/svc/milestone/identity-node:62
Style: use [[ ]] not [ ]. Also, you can use boolean logic in the
test. IOW:
if [[ -z "$hostname" -s /etc/nodename ]]; then
- usr/src/cmd/svc/milestone/identity-node:64
Mind your quotes!! Make this:
$SVCCFG -s $SVC setprop config/nodename = astring: "$hostname"
- usr/src/cmd/svc/milestone/identity-node:65
Style: use [[ ]] instead of [ ], or better, do this:
if $SVCCFG -s $SVC setprop config/nodename = astring: "$hostname"; then
...
else
...
fi
- usr/src/cmd/svc/milestone/identity-node:66
Echo is built-in...
- usr/src/cmd/svc/milestone/identity-node:70
Style: better set PATH explicitly than have to qualify every command...
- usr/src/cmd/svc/milestone/identity-node
It'd be nice if you could convert the rest of the script to use
modern ksh-ims.
- usr/src/cmd/cvcd/sparc/sun4u/starfire/cvcd.c:206
Style: not your code, but, I prefer to not use '!' to check == 0:
204 206 if (strlen(hostname) == 0) {
instead of
204 206 if (!strlen(hostname)) {
- Should other commands' usage messages be updated too? E.g., uname(1M)'s.
Nico
--
_______________________________________________
networking-discuss mailing list
[email protected]
