Hi Sanjay,
My apologies for not seeing your code-review earlier. My responses
in-line.
Thank you,
Clay
On Wed, 14 Jul 2010, sanjay nadkarni wrote:
Trying to avoid repeating the same issues hence including John's response. So
here's my review:
installadm.c:
491-495: I will be a bit more explicit and suggest that the comments that
describe the overall functionality should be included at the top.
This has all been simplified to use a function in common with the 5
installadm shell-scripts which get networks to use as well.
Another option to setting a global variable for multihomed is to have static
variable in the function that is set appropriately. i.e. start with -1 and
then set it to 0 or 1. Makes it a bit more OO.
640: if (is_multihomed() != B_FALSE)
Better if framed as positive:
if (is_multihomed() == B_TRUE)
Fixed
manifest-locator:
116 if [ -n "$(print "$AI_SERVICE_ADDRESS" | $GREP '$serverIP')" ];
then
117 AI_SERVICE_ADDRESS=$(print $AI_SERVICE_ADDRESS | \
118 $SED "s/\$serverIP/$($DHCPINFO $BOOTSERVER)/")
There has to be a better way to do this. If it requires setting more
variables and steps that is fine. But this is clearly a case of code
obfuscation ;-)
I think the obfuscation has been replaced with ksh-ification:
113 #
114 # Check to see if AI_SERVICE_ADDRESS has the $serverIP keyword
115 #
116 if [[ "${$AI_SERVICE_ADDRESS/\$serverIP/}" !=
117 "$AI_SERVICE_ADDRESS" ]]; then
118 # replace the $serverIP keyword with the machine's
119 # boot server IP address
120 AI_SERVICE_ADDRESS=\
121 "${$AI_SERVICE_ADDRESS/\$serverIP/$($DHCPINFO
$BOOTSERVER)}"
[snip...]
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
