Quaker,
In general, the changes look good. I have few comments, mostly nits.
Thanks,
-Tony
usr/src/Makefile - OK
usr/src/cmd/Makefile - OK
/usr/src/cmd/cmd-inet/usr.lib/Makefile - OK
usr/src/lib/libwladm/Makefile.com - OK
For the packaging changes, I only look at the SMF side of things since
we're not packaging experts :^) Basically, I look at the following
- manifests type "f" and use the 'manifest' class
- i.manifest and r.manifest included properly. If the service
isn't in ON, the packages are constructed to use the bundled
i.manifest and r.manifest, rather than creating private copies.
- if new service should be enabled on fresh install(in a profile), an
appropriate postinstall should be there.
- if conversion of an existing service (inetd.conf or init.d
script), a combination of preinstall and postinstall scripts
may be necessary to ensure the service remains enabled after
upgrade.
usr/src/pkgdefs/Makefile - looks fine
usr/src/pkgdefs/SUNWsupr/Makefile - looks fine
usr/src/pkgdefs/SUNWsupr/depend - looks fine
usr/src/pkgdefs/SUNWsupr/pkginfo.tmpl - looks fine
usr/src/pkgdefs/SUNWsupr/prototype_com - looks fine
usr/src/pkgdefs/SUNWsupr/prototype_i386 - looks fine
usr/src/pkgdefs/SUNWsupu/Makefile - looks fine
usr/src/pkgdefs/SUNWsupu/depend - looks fine
usr/src/pkgdefs/SUNWsupu/pkginfo.tmpl - looks fine
usr/src/pkgdefs/SUNWsupu/prototype_com - looks fine
usr/src/pkgdefs/SUNWsupu/prototype_i386 - looks fine
usr/src/cmd/cmd-inet/usr.lib/wpad/svc-wpa
Don't you want to include the CDDL header in this file?
- Can you give more explanations as to how both SMF methods and
sys-unconfig/sysidconfig use this script? Probably expand on the
different arguments?
usr/src/cmd/cmd-inet/usr.lib/wpad/wpa.xml - OK
usr/src/lib/libwladm/common/libwladm.c
add_new_property - we're not consistent in calling scf_*_destroy
functions to clean up here when there's a failure. Assuming the command
will exit, you can choose to leave the code as it is or modify such that
we do the appropriate cleaning up.
add_pg_method - calling scf_transaction_reset is redundant here since
scf_transaction_destroy* functions effectively calls
scf_transaction_reset. Also, looks like the "goto out2" statements in
case rv is not 0 can be change to "break" and the out sections can
probably be simplified to
out:
if (trans != NULL) {
scf_transaction_destroy_children(tran);
scf_transaction_destroy(tran);
}
if (pg != NULL)
scf_pg_destroy(pg);
return (status)
create_instance - nits
line 2219 - calling "goto out" here would be more consistent.
create_service - this name is a bit misleading since we're not creating
a new service but simply setting things up before creating an instance.
Is there a reason why the code can't be a part of
create_instance(similar to delete_instance)?
- Again, the multiple goto sections can be simplified if we check for
the non-null value and call the appropriate destroy function.
- Line 2248 seems unnecessary since we can use SERVICE_NAME definition
in scf_handle_decode_fmri call.
wpa_instance_create - Is username always "root"?
wait_till_to - what happens when max is not assigned a valid value since
one of the clauses in the if statement fails?
delete_instance - again the out sections can be simplified
line 2364 - SERVICE_NAME can be used as a parameter
line 2407 - I don't understand this comment
