Hi Sue,

thank you very much for review.
Please see my response in line.

Jan


On 06/16/10 12:42 AM, Sue Sohn wrote:
Hi Jan,

The updated webrev is available at
http://cr.opensolaris.org/~dambi/bug-15723-cr/
Delta webrev
http://cr.opensolaris.org/~dambi/bug-15723-cr-delta/

auto_parse.c
62 Comment says catching stderr for debugging but I'm not seeing where this is done

It is copy-paste issue - I have moved that comment to the right place and changed
it accordingly. Thank you for catching this.


svc-system-config
276(and other functions) Perhaps add error checking to verify you have expected number of passed in parameters?

To be honest, I am not quite convinced it would add value -
as all those functions are 'private' (not public API), potential
discrepancies would be captured during testing phase.
However, if you think it is worth to add those checks, please
let me know, I will do that.

369 Print statement has typo: Failed to set owneship _> Failed to set ownership

I have changed it.


sc_conv.ksh
30 delivered into -> part of the

I have changed it.

30 untilized -> utilized

I have changed it.

185 Does this script need to be run as root (if so, add check)?

No, root privileges are not needed.

192 If manifest_new already exists, do you want to give warning to user?

Yep. I have added warning message.

296 Don't know that you need the word "aborting" in this message since it's fairly minor and you're done anyway.

You are right. I have removed it.

I have updated webrevs accordingly:
http://cr.opensolaris.org/~dambi/bug-15723-cr/
http://cr.opensolaris.org/~dambi/bug-15723-cr-delta/

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to