Hi Joe,
Thank you for reviewing the changes. I fixed all the issues
in boot_archive_configure and live_fs_include.sh.
Tony will be responding to your comments in mkrepo.
Thanks,
--Karen
On 03/24/10 10:56, Joseph J VLcek wrote:
>
> Hey Karen,
>
> Issue 2 and 7 should be addressed.
>
> The others are nit but I think would be nice to have.
>
> Joe
>
>
>
>
> Issue 1 (nit):
> --------
> usr/src/cmd/distro_const/utils/boot_archive_configure
>
>
> Total nit: maybe not all that important but that I would point it out...
>
> Use "==" not "="
>
> 260 [ "${emi}" = "true" -a ${etc_profile} -ne 0 ] && continue
> 261 [ "${emi}" = "false" -a ${etc_profile} -eq 0 ] && continue
>
>
> From ksh93(1):
> string = pattern Same as ==, but is obsolete.
>
>
> Issue 2:
> --------
>
> http://installzone-wiki.central.sun.com/wiki/index.php/Ksh93_Tips
> # 9 use true and false ksh93 builtins
>
> I think this could work but it is wrong and should be changed.
>
> 260 [ "${emi}" = "true" -a ${etc_profile} -ne 0 ] && continue
> 261 [ "${emi}" = "false" -a ${etc_profile} -eq 0 ] && continue
>
> You should not check against the strings "true" and "false". You
> should check against the builtins true and false
>
> e.g.:
>
> 260 [ ${emi} -a ${etc_profile} -ne 0 ] && continue
> 261 [ ${emi} -a ${etc_profile} -eq 0 ] && continue
>
> Issue 3 (nit):
> -------
>
> 259 etc_profile=$?
>
> etc_profile should be an integer
>
> e.g.:
>
> typeset -i etc_profile at the top of the program
>
>
> Issue 4 (nit):
> --------
> usr/src/cmd/distro_const/utils/mkrepo
>
> nit:
>
> 120 rm -f $logf
>
> rm should be defined as a builtin before being used.
>
> e.g.
>
> add this line to the top of the file:
>
> builtin rm
>
> Issue 5 (nit):
> --------
>
> Same issue with grep, cat and echo
>
> 129 grep "smf(5) service descriptions failed to load"
> $logf > /dev/null 2>&1
>
>
> Issue 6 (nit):
> --------
>
> fgrep might be a better choice then grep
>
> Issue 7:
> --------
>
> http://installzone-wiki.central.sun.com/wiki/index.php/Ksh93_Tips
> # 4 Use "print" and "printf" in place of "echo"
>
>
>
> Issue 8 (nit):
> --------
>
> usr/src/cmd/slim-install/svc/live_fs_include.sh
>
> Nit:
>
> use
>
> builtin cd
>
>
>
>
>
>
>
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
