Hi Evan,
please see below for libaiscf comments.
Thank you,
Jan
libaiscf/Makefile
-----------------
general comment - I can see that mix of ${} and $()
is used for variables - I might recommend to pick
one of them - perhaps $(), since it seems to be used
more and also in other Makefiles.
42 - might this line be removed ?
27,69,72 - since SUBDIRS is empty, I think this
code could be removed and 59 simplified:
59 all: $(HDRS) static dynamic .WAIT $(SUBDIRS)
->
59 all: $(HDRS) static dynamic
ai_create.c
-----------
since there is only ai_delete_install_service() function
defined there, it might be better to use ai_delete.c
name of the file instead of ai_create.c
55 - according to comment at 51, it might be better if function
returns ai_errno_t instead of int.
74 - ai_get_instance() might be used instead
ai_trans.c
----------
60, 126, 156 - it might be better to return ai_errno_t instead of int
On 03/27/09 16:17, Evan Layton wrote:
> Thanks Jan!
>
> We'd like to get this done by close of business on Tuesday if possible
> so we have some time to make the changes from the comments we receive.
>
> -evan
>
> jan damborsky wrote:
>> Hi Evan,
>>
>> I could take a look at libaiscf library.
>> When you would like to have it done ?
>>
>> Jan
>>
>>
>> On 03/26/09 22:41, 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
>>
>
