Jan,
The changes look good now.
- Sundar
jan damborsky wrote:
> Hi Sundar,
>
> thank you very much for your comments.
> Please see my response in line.
> The webrev has been updated with changes.
>
> Jan
>
>
>
> On 04/03/09 18:58, Sundar Yamunachari wrote:
>> Jan,
>> *
>> usr/src/cmd/auto-install/auto_install.c*
>>
>> Can you change enable_debug_mode() to set_debug_mode() and
>> is_debug_mode_enabled() to get_debug_mode()? This will make it more
>> clear and if want to allow setting more than one debug value in the
>> future, it will be simpler.
>
> As far as support for more debug levels is concerned,
> I agree with Dave. As implemented right now, there
> is no common design for supporting more debug levels
> right now across installer technologies
> (e.g. beadm module has only one level).
> Also, as we have experienced so far, once we are interested
> in debug messages (e.g. when evaluating some issue), we
> have always asked for turning on the most verbose level
> in order to obtain as much information as possible.
> Setting intermediate levels has not been used so far.
> Based on current state of things, it seems unlikely
> that we might need that feature in near future.
> Please let me know what you think.
>
>>
>> 1582: Add a break after this line. What about having a default case
>> to display usage and exit if the user specifies any other option?
>
> Good catch ! I have added break and usage message
> is displayed if incorrect options are provided.
>
>>
>> 1615-1626: Needed only if debug_is_enabled. Can you move it with in
>> the block 1597-1613?
>
> We need to call ls_init() for initializing logging service
> even if debug mode is not enabled - in that case it is invoked
> will NULL parameter.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20090406/e34b0936/attachment.html>
