Hi Evan, I have only one comment for libaiscf/Makefile:
51 CFLAGS += $(DEBUG_CFLAGS) -Xa $(CPPFLAGS)} -> 51 CFLAGS += $(DEBUG_CFLAGS) -Xa $(CPPFLAGS) Other than that the changes look good. No need for another webrev :-) Thank you, Jan On 04/01/09 18:04, Evan Layton wrote: > 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 >> >
