Joe,

Thanks for the review...


Joseph J. VLcek wrote:
> Ethan Quach wrote:
>> These are really simple ones...
>>
>>
>> webrev:
>> ------------
>> http://cr.opensolaris.org/~equach/webrev.8908.9173.9842
>>
>>
>> Bugids:
>> -----------
>> http://defect.opensolaris.org/bz/show_bug.cgi?id=8908
>> http://defect.opensolaris.org/bz/show_bug.cgi?id=9173
>> http://defect.opensolaris.org/bz/show_bug.cgi?id=9842
>>
>>
>> thanks,
>> -ethan
>>
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>
> Looks good Ethan
>
> I only have 1 question:
> In usr/src/cmd/installadm/setup-image.sh do you want to direct stderr 
> to /dev/null?
>
> e.g.:
>  371                         umount ${caid_mnt} > /dev/null
>  372                         lofiadm -d $caid_lofi_dev > /dev/null
>  373                         rmdir ${caid_mnt} > /dev/null
>  374                         rmdir ${img_ai_dir} > /dev/null
>
> to:
>  371                         umount ${caid_mnt} > /dev/null 2>&1
>  372                         lofiadm -d $caid_lofi_dev > /dev/null 2>&1
>  373                         rmdir ${caid_mnt} > /dev/null 2>&1
>  374                         rmdir ${img_ai_dir} > /dev/null 2>&1
>
> The entire  function check_auto_install_dir() does not do this. 
> Shouldn't it?

This was asked in another review and we came to
the consensus that stderr should pop out to the screen
so that the user can see the error.


thanks,
-ethan

Reply via email to