Subject: kauth_cred design needs work
To: Elad Efrat <elad@NetBSD.org>
From: Darren Reed <darrenr@NetBSD.org>
List: tech-kern
Date: 03/20/2006 07:51:02
On Mon, Mar 20, 2006 at 12:42:49AM +0200, Elad Efrat wrote:
...
> Two links that may be of interest:
> http://www.bsd.org.il/netbsd/kauth.9.html - online man-page

Ok, I've had a look at it and I think the design needs more work,
so I'm moving this from develoeprs to tech-kern.

You have a kauth_cred_getrefcnt().  Why?

I've never seen anything like this in any design I've looked at before,
so I'm really puzzled that you have it in yours (this is the red flag
that indicates the design needs work.)  Is this there because Apple
has something similar?

Why do you want to return the refcnt?  Why do you care if the value
is 4 or 5?  Do you care if it is anything other than 0 or > 0 ?
In which case, shouldn't you have a function returning a boolean
called "kauth_cred_isheld()" ?  Then again, why should code outside
of the implementation care about whether or not it is held?

You have both kauth_cred_hold() and kauth_cred_dup().  Why ?
Why isn't the former just a macro?
i.e.
#define  kauth_cred_hold(x)   (void) kauth_cred_dup(x)

Maybe the programming that uses kauth needs some more work to see
if all uses of kauth_cred_hold can be replaced with kauth_cred_dup.

i.e. rather than doing:

kauth_t *k;

k = something->kauth;
kauth_cred_hold(k)

do

kauth_t *k = kauth_cred_dup(something->kauth)

If kauth_cred_destroy() is never called directly, remove it from
the man page and update the description of _free()

In the description of kauth_cred_copy, when you say clone, do you
mean you call kauth_cred_clone() ?  Otherwise, what is the purpose
of this function?  The description seems, to me, confused, with
the "reference count of 1" special case.

I think you need to rework the entire document (and design?) where
it concerns the management of the kauth_t objects themselves.

It most definately is not ready for putting into -current but
you should create a branch and put it on the branch so others
can see the work in progress.

Darren