Index: libabac/abac_keyid_map.c
===================================================================
--- libabac/abac_keyid_map.c	(revision 6cd69a0d7919d36d4cc937ae13ce7f09034097d5)
+++ libabac/abac_keyid_map.c	(revision 154696257918dfe1e9ac3f57b30169b61f0f1670)
@@ -6,4 +6,8 @@
 #include "uthash.h"
 
+/*
+ * A mapping entry that maps key to valus (both char *s).  These can be hashed
+ * because of the UT_hash_handle - see uthash.h.
+ */
 struct abac_keyid_mapping_t {
     char *key;
@@ -12,10 +16,18 @@
 };
 
+/* 
+ * A map from keyids -> nicknames and nicknames to keys.  These are managed by
+ * the libabac reference counting system, hence the refcount
+ */
 struct abac_keyid_map_t {
-    abac_keyid_mapping_t *keys;
-    abac_keyid_mapping_t *nicknames;
+    abac_keyid_mapping_t *keys;		/* Key to nickname map */
+    abac_keyid_mapping_t *nicknames;	/* Nickname to key map */
     int refcount;
 };
 
+/*
+ * Create a new mapping entry from key(k) to value (v).  It must be freed using
+ * abac_keyid_mapping_free.
+ */
 abac_keyid_mapping_t *abac_keyid_mapping_new(char *k, char *v) {
     abac_keyid_mapping_t *m = NULL;
@@ -29,4 +41,8 @@
 }
 
+/*
+ * Free the given mapping.  These are not reference counted, so free the key
+ * and value, and then the mapping memory
+ */
 void abac_keyid_mapping_free(abac_keyid_mapping_t *m) {
     if ( m->key ) free(m->key);
@@ -35,4 +51,8 @@
 }
 
+/*
+ * Create a new keyid map.  These are reference counted and must be freed using
+ * abac_keyid_map_free.
+ */
 abac_keyid_map_t *abac_keyid_map_new() {
     abac_keyid_map_t *m = abac_xmalloc(sizeof(abac_keyid_map_t));
@@ -44,5 +64,8 @@
 }
 
-/* Make a new, independent copy of the old keymap. */
+/* 
+ * Make a new, independent copy of the old keymap. This allocates new memory
+ * with a new reference count.  abac_keyid_map_free must be called on it.
+ */
 abac_keyid_map_t *abac_keyid_map_clone(abac_keyid_map_t *old) {
     abac_keyid_map_t *m = abac_xmalloc(sizeof(abac_keyid_map_t));
@@ -59,5 +82,9 @@
 }
 
-/* Add a reference to old and return it. */
+/* 
+ * Add a reference to the old map and return it.  The reference count has been
+ * incremented, so the underlying memory will not be freed until
+ * abac_keyid_map_free is called once for each reference.
+ */
 abac_keyid_map_t *abac_keyid_map_dup(abac_keyid_map_t *old) {
     old->refcount ++;
@@ -65,4 +92,8 @@
 }
 
+/*
+ * Free the reference-counted map m.  Decrement the reference count.  If and
+ * only if the reference count is 0 or less, delete all the associated memory.
+ */
 void abac_keyid_map_free(abac_keyid_map_t *m) {
     abac_keyid_mapping_t *me = NULL;
@@ -81,4 +112,8 @@
 }
 
+/*
+ * Return the nickname associated with this keyid, if nay.  The caller is
+ * responsible for freeing the returned string.
+ */
 char *abac_keyid_map_key_to_nickname(abac_keyid_map_t *m, char *key) {
     abac_keyid_mapping_t *me = NULL;
@@ -91,4 +126,8 @@
 }
 
+/*
+ * Return the keyid associated with this nickname, if nay.  The caller is
+ * responsible for freeing the returned string.
+ */
 char *abac_keyid_map_nickname_to_key(abac_keyid_map_t *m, char *nick) {
     abac_keyid_mapping_t *me = NULL;
@@ -101,4 +140,7 @@
 }
 
+/*
+ * Remove this keyid from both mappings.
+ */
 int abac_keyid_map_remove_keyid(abac_keyid_map_t *m, char *key) {
     abac_keyid_mapping_t *me = NULL;
@@ -119,12 +161,21 @@
 }
 
-
+/*
+ * If this keyid is not mapped to a nickname, add a mapping from key to
+ * nickname.  If the nickname is already assigned to a key, disambiguate it by
+ * adding trailing numbers.  If more than 1000 tries are made to disambiguate,
+ * give up.
+ */
 int abac_keyid_map_add_nickname(abac_keyid_map_t *m, char *key, char *nick) {
     abac_keyid_mapping_t *me = NULL;
     char *name = NULL;
+    char *p = NULL;
     int i =0;
 
     if ( !key || !nick) return 0;
-    if (abac_keyid_map_key_to_nickname(m, key)) return 0;
+    if ( (p = abac_keyid_map_key_to_nickname(m, key))) {
+	free(p);
+	return 0;
+    }
 
     if ( !(name = abac_xmalloc(strlen(nick)+10))) return 0;
@@ -141,7 +192,13 @@
     }
     free(name);
-    return 1;
-}
-
+    return (i < 1000);
+}
+
+/*
+ * Merge the mappings in src to the mappings in dest.  If overwrite is true,
+ * src mappings always overwrite dest mappings, otherwise the dest mappings
+ * remain.  Calls abac_keyid_map_add_nickname internally, so nicknames in src
+ * that are also in dest are disambiguated.
+ */
 void abac_keyid_map_merge(abac_keyid_map_t *dest, abac_keyid_map_t *src, 
 	int overwrite) {
@@ -163,4 +220,7 @@
 }
 
+/*
+ * Utility function to identify separator characters in a role.
+ */
 static int is_sep(char *c) {
     switch (*c) {
@@ -174,4 +234,11 @@
 }
 
+/*
+ * Break s into sections by separator characters (see is_sep) and copy them
+ * into a return value.  If lookup finds a replacement for a section, the
+ * replacement is used, otherwise the original string is used.  Using different
+ * lookup functions implements expand_key and expand_name.  In either case, the
+ * caller is responsible for freeing the returned value.
+ */
 static char *abac_keyid_map_replace(abac_keyid_map_t *m, char *s, 
 	char *(*lookup)(abac_keyid_map_t *, char *)) {
@@ -230,8 +297,16 @@
 }
 
+/*
+ * Break s up into sections and replace any keyids that have nicknames with
+ * those nicknames.  The caller is responsible for freeing the return value.
+ */
 char *abac_keyid_map_expand_key(abac_keyid_map_t *m, char *s) {
     return abac_keyid_map_replace(m , s, abac_keyid_map_key_to_nickname);
 }
 
+/*
+ * Break s up into sections and replace any nicknames that have keyids with
+ * those keyids.  The caller is responsible for freeing the return value.
+ */
 char *abac_keyid_map_expand_nickname(abac_keyid_map_t *m, char *s) {
     return abac_keyid_map_replace(m , s, abac_keyid_map_nickname_to_key);
