Quoth Tom Whitten on Wed, Dec 03, 2008 at 10:43:29AM -0700:
> I've posted a webrev at
> http://cr.opensolaris.org/~fourctom/audit_02_dec_2008/ that contains a
> number of fixes for bugs in the SMF audit code. Specifically, there are
cmd/svc/configd/client.c
start_audit_session(): Won't this code still produce a syslog LOG_ERR
message when door_ucred() fails with EINVAL? Why not just return
instead? Or set cp->rc_adt_session and return?
cmd/svc/configd/rc_node.c
perm_status_t: I'm a little concerned that the PG_ prefix is too
reminiscent of property groups, and it may be better to use a prefix
like PERM_. What do you think?
map_granted_status(): I don't see where this function is used outside
this file. If it's not, please make it static.
RC_NODE_CHECK_AND_LOCK_OR_FREE_RETURN(): This macro is only used once.
Are you married to it? Just invoking rc_node_check_and_lock()
directly at 4021 should save some developer complexity, and probably
shorten rc_node.c.
3993: Can you add a comment explaining that we continue on PG_DENIED
so we can fill fmri if auditing is on?
4001-2: Please use bad_error() here.
5139-48: Are you married to this change? I think just calling
rc_node_hold_flag() directly would be easier to maintain.
5318-9: Please use bad_error() here.
5503: Please add a comment explaining why we continue on PG_DENIED.
5953: Why is this needed here but not elsewhere?
cmd/svc/svccfg/svccfg_libscf.c
7017: Why did you change this? Is the fix in lowlevel.c not
sufficient?
lib/libscf/common/lowlevel.c
7295: I suspect that in the long-run it's not a good idea to return
the success code when the function didn't do what it's supposed to
do. Is there some reason that doesn't apply in this case?
David
