On 03/24/10 15:58, Karen Tung wrote: > Hi Joe, > > I indicated that I will fix all issues you suggested in the > boot_archive_configure script. However, after putting > in your suggested fixes and testing, I was not able to > incorporate your issues 2 and 3 below. > > I took your suggested fixes, but they don't work. > I played around with it, but was not able to get it > to work. > > Since I must integrate today, and this does not affect > the correctness of the code, I think it would be best > for me to integrate right now, and file a bug to > make this change at a later time.
Hi Joe, I filed the following bug to keep track of the 2 issues that I am not able to address: http://defect.opensolaris.org/bz/show_bug.cgi?id=15347 Thanks, --Karen > > Thanks, > > --Karen > > On 03/24/10 12:47, Karen Tung wrote: >> 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 >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
