Looks fine.

sarah
****
> I've created a new webrev with these changes as well as some comment
> clean-up that was missed for get_service_props() and set_service_props()
> in installadm_util.c. We also changed check_for_enabled_install_services
> in installadm.c so that if the service is already in maintenance mode we
> print out a mesage and return skipping the code that would attempt to
> put the smf service into maintenance.
>
> The new webrev is available at:
> http://cr.opensolaris.org/~evanl/7218.02/
>
> Thanks!
>
> -evan
>
>
> jan damborsky wrote:
>> Hi Evan,
>>
>> I have only couple of nits - please see below.
>>
>> Thank you,
>> Jan
>>
>>
>> libaiscf/Makefile
>> -----------------
>> 50,51 - please change ${} -> $()
>>
>> ai_utils.c
>> ----------
>> 454         name = malloc(namelen);
>> 455         valuestr = malloc(vallen);
>> ->
>> 454         name = malloc(namelen + 1);
>> 455         valuestr = malloc(vallen + 1);
>>
>> 578         buff = malloc(namelen);
>> ->
>> 578         buff = malloc(namelen + 1);
>>
>>
>>
>>
>> On 04/01/09 01:01, Evan Layton wrote:
>>> This webrev didn't have the fix for calling create-service more than 
>>> once on the save AI service name. The fix was to simply return 
>>> success if a property group already exists for the AI service name.
>>>
>>> I've updated the webrev to include the fix for this.
>>>
>>> Thanks,
>>> -evan
>>>
>>> Evan Layton wrote:
>>>> The source has been updated with all the changes made from the code 
>>>> review comments up to this point. I've created a new webrev which 
>>>> contains these changes.
>>>> http://cr.opensolaris.org/~evanl/7218.01
>>>>
>>>> Thanks for all the comments!
>>>>
>>>> -evan
>>>>
>>>> Evan Layton wrote:
>>>>> My apologies for the delay. I have fixed the problems with the 
>>>>> mis-merge, sanity tested the changes and updated the webrev. I've 
>>>>> listed out the files and split things up between the library and 
>>>>> installadm. Just pick a subset of files and let me know what 
>>>>> you're looking at som we make sure everyting gets covered.
>>>>>
>>>>> And thanks again to Sue for catching the mis-merge!!!
>>>>>
>>>>> Thanks!
>>>>> -evan
>>>>>
>>>>> installadm files:
>>>>> usr/src/cmd/installadm/Makefile
>>>>> usr/src/cmd/installadm/installadm-common.sh
>>>>> usr/src/cmd/installadm/installadm.c
>>>>> usr/src/cmd/installadm/installadm.h
>>>>> usr/src/cmd/installadm/installadm_util.c
>>>>> usr/src/cmd/installadm/server.xml
>>>>> usr/src/cmd/installadm/setup-service.sh
>>>>> usr/src/cmd/installadm/svc-install-server
>>>>> usr/src/pkgdefs/SUNWinstall-libs/prototype_com
>>>>>
>>>>> libaiscf Library files:
>>>>> usr/src/lib/Makefile
>>>>> usr/src/lib/libaiscf/Makefile
>>>>> usr/src/lib/libaiscf/ai_create.c
>>>>> usr/src/lib/libaiscf/ai_trans.c
>>>>> usr/src/lib/libaiscf/ai_utils.c
>>>>> usr/src/lib/libaiscf/libaiscf.h
>>>>>
>>>>>
>>>>> And if anybody is REALLY board here's the test files...
>>>>>
>>>>> optional files:
>>>>> usr/src/cmd/installadm/test/installadm_test.c
>>>>> usr/src/cmd/installadm/test/manual_tests
>>>>>
>>>>>
>>>>> Evan Layton wrote:
>>>>>> Sue pointed out that the webrev shows that I have a mis-merge so 
>>>>>> if you're looking at the changes please stop. I'll get the 
>>>>>> mis-merge fixed and send out a new webrev as soon as possible.
>>>>>>
>>>>>> Thanks,
>>>>>> -evan
>>>>>>
>>>>>>
>>>>>> Evan Layton wrote:
>>>>>>>
>>>>>>> We need to get the fix for 7218 code reviewed. There are around 
>>>>>>> 2000 lines of changes and new code so we'll need volunteers that 
>>>>>>> can take sections of the code to look at. I'm sending this out 
>>>>>>> now so the link is out there but I'll want to get things divided 
>>>>>>> up tomorrow morning and get folks looking at this tomorrow.
>>>>>>>
>>>>>>> The bug is 7218
>>>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7218
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.opensolaris.org/~evanl/7218/
>>>>>>>
>>>>>>> Thanks!
>>>>>>> -evan
>>>>>>> _______________________________________________
>>>>>>> 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
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> 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


Reply via email to