Hi Sue.
Thanks for reviewing. I implemented all of your suggestions.
Webrev updated.
On 03/11/09 16:23, Susan Sohn wrote:
> Hi Jack,
>
> installadm.c
> 309 - Since MSG_BAD_SERVICE_NAME contains a \n, won't you be printing
> two \n's?
Thanks. Yes, I'll fix this everywhere I do this.
> 306,318 - Why are there curly brackets around the case statement? It
> looks kind of odd and doesn't match the rest of the case statement.
> And the closing bracket is in the wrong place anyway, because it
> encompasses the comment from the next case.
They were leftover from my earlier fix. Corrected.
> 680,756,874, etc. - same comment as 309
Fixed.
> 1033 - Maybe validate the service name at 1061, instead? This would
> maintain consistency with how you did it in do_add and do_remove.
OK.
> 1310 - I'd remove the XXX, I don't think it adds anything to the comment.
OK.
Thanks,
Jack
>
> Thanks,
> Sue
>
> On 03/11/09 14:00, Jack Schwartz wrote:
>> Hi everyone.
>>
>> Fix has been renovated with the following:
>> - Only alphanumeric chars, underscore and hyphen allowed in service
>> names, per our bugcourt meeting yesterday.
>> - I fixed that the service name wasn't checked everywhere necessary.
>> - Shortened several lines > 80 chars and other stuff like that to get
>> code hg nits clean.
>> - Other cleanup.
>>
>> Same location:
>> http://cr.opensolaris.org/~schwartz/090306.1/webrev/
>>
>> Please review.
>>
>> Thanks,
>> Jack
>>
>> On 03/06/09 13:41, Jack Schwartz wrote:
>>> Hi everyone.
>>>
>>> Here is a code review for a couple of small bugfixes:
>>>
>>> 5091 AI install does not work if your service name had . in it.
>>> 4610 most installadm commands need to err out gracefully if not root
>>>
>>> http://cr.opensolaris.org/~schwartz/090306.1/webrev/
>>>
>>> Please review.
>>>
>>> Thanks,
>>> Jack
>>> _______________________________________________
>>> 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
>
