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.
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
