Jack,
Looks ok now.
Sue
Jack Schwartz wrote:
> 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
>>
>
