Hi Sue.
On 01/15/09 13:32, Susan Sohn wrote:
> Hi Jack,
>
> Thanks for the review.
>
> Jack Schwartz wrote:
>> Hi Sue.
>>
>> Thanks for taking care of this. Here is my feedback:
>> *
>> *Un-explicitly-listed files: OK
>> *
>> *manifestfile.html:
>>
>> Hmmm... There are refs to OpenSolaris 2008.11in this doc and we are
>> making this change after that release... Changing bootroot
>> references here introduce a sort of inconsistency w.r.t. the release.
>> Still, this should be OK because the SPARC effort is just for a
>> development build. The release will be changed, I'm sure, before the
>> code is officially released.
>>
>> So... OK.
>>
>> installadm.c:
>>
>> 372: Yes, I know this code is correct, but I would prefer instead of
>> if (! stat() && ... )
>> to see
>> if (stat == 0) && ...
>
> ok
>
>> because stat returns an int and because people won't have to go to
>> K&R and look up that ! is higher precedence than &&.
>>
>> 378: MSG_UNABLE_TO_DETERMINE_TYPE can be made more explicit by
>> calling it MSG_UNABLE_TO_DETERMINE_ARCH.
>
> ok
>
>> installadm.h:
>>
>> 115: same request as above for installadm.c
>>
>> 116: OK, so I see why you used "type". Still I think "arch" is
>> appropriate and more explicit.
It still says "install image type" but I've reconsidered and this will
be a more meaningful message than "install image arch" so leave it.
Ship it!
Thanks,
Jack
>
> ok, I'll change this.
>
> webrev updated, same location.
>
> Thanks,
> Sue
>
>> Thanks,
>> Jack
>>
>>
>>
>> On 01/15/09 11:27, Susan Sohn wrote:
>>> Please review the changes for:
>>>
>>> 5787 microroot name shouldn't be ISA specific
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5787
>>>
>>> and
>>>
>>> 5789 Sticky bit on bootroot file
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5789
>>>
>>> which are posted at:
>>>
>>> http://cr.opensolaris.org/~sohn/5787_5789
>>>
>>> Thanks,
>>> Sue
>>>
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>
>>
>
