Re: Support for multi-position electro-mechanical keylocks

[Arnaud Lacombe <lacombar%gmail.com@localhost>]

On Sun, Aug 2, 2009 at 5:54 AM, Marc Balmer<marc%msys.ch@localhost> wrote:
> The attached diff, which is also online at
> http://www.netbsd.org/~mbalmer/diffs/keylock_03.diff, adds the following
> components:

From a quick look (mostly style):
+file   kern/kern_keylock.c             keylock
Why put that in /kern ? [certainly a good one depending of what
multi-position keylock are]
+struct gpiolock_softc {
[...]
+       int                     __map[GPIOLOCK_MAXPINS];
+
shouldn't this be fixed length ?

+
+int (*keylock_pos_cb)(void *) = NULL;
+void *keylock_pos_cb_arg = NULL;
I'm not a big fan of such publicly, globally accessible callback.
Especially in a security context. Moreover, if it is not intended to
be accessed out of the file, make them static.

+int secmodel_keylock_system_cb(kauth_cred_t, kauth_action_t, void *,
+    void *, void *, void *, void *);
+int secmodel_keylock_process_cb(kauth_cred_t, kauth_action_t, void *,
+    void *, void *, void *, void *);
+int secmodel_keylock_network_cb(kauth_cred_t, kauth_action_t, void *,
+    void *, void *, void *, void *);
+int secmodel_keylock_machdep_cb(kauth_cred_t, kauth_action_t, void *,
+    void *, void *, void *, void *);
+int secmodel_keylock_device_cb(kauth_cred_t, kauth_action_t, void *,
+    void *, void *, void *, void *);
+
Same as before, define these in 'keylock/secmodel_keylock.c' and make
them static.

 - Arnaud