- learn to iterate over maps

- rename visited lists to related lists
- handle NULL related ID lists
- fix map semantics so that it won't try to take ownership of key/value data
- set up map backend data
- start doing proper adds/removes in post-op callbacks

4 files changed, 268 insertions, 68 deletions

diff --git a/src/backend.c b/src/backend.c
index b52b7f0..28f6275 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -17,51 +17,106 @@
 #include "map.h"
 #include "defaults.h"
 
-/* Given a map/domain entry, add it to the map. */
+/* The data we ask the map cache to keep, for us, for each map. */
 struct backend_map_entry_cb_data {
 	struct plugin_state *state;
-	const char *domain, *map, *key, *value;
+	char *domain, *map, *key_attr, *value_fmt;
 };
 
+static struct backend_map_entry_cb_data *
+backend_copy_cb_data(const struct backend_map_entry_cb_data *data)
+{
+	struct backend_map_entry_cb_data *ret;
+	ret = malloc(sizeof(*ret));
+	if (ret == NULL) {
+		return NULL;
+	}
+	ret->state = data->state;
+	ret->domain = strdup(data->domain);
+	ret->map = strdup(data->map);
+	ret->key_attr = strdup(data->key_attr);
+	ret->value_fmt = strdup(data->value_fmt);
+	if ((ret->domain == NULL) ||
+	    (ret->map == NULL) ||
+	    (ret->key_attr == NULL) ||
+	    (ret->value_fmt == NULL)) {
+		free(ret->domain);
+		free(ret->map);
+		free(ret->key_attr);
+		free(ret->value_fmt);
+		free(ret);
+		return NULL;
+	}
+	return ret;
+}
+
+static void
+backend_free_cb_data(void *data)
+{
+	struct backend_map_entry_cb_data *cb_data = data;
+	if (cb_data != NULL) {
+		free(cb_data->domain);
+		free(cb_data->map);
+		free(cb_data->key_attr);
+		free(cb_data->value_fmt);
+		free(cb_data);
+	}
+}
+
+/* Given a map-entry directory entry, determine which keys it should have,
+ * determine which value should be associated with those keys, and add them to
+ * the map cache. */
 static int
 backend_map_entry_cb(Slapi_Entry *e, void *callback_data)
 {
 	struct backend_map_entry_cb_data *data;
-	char **key, **value, *dn;
+	char *key, *value, *dn, **visited_dn;
 	int i, j;
 	data = callback_data;
-	key = slapi_entry_attr_get_charray(e, data->key);
-	value = slapi_entry_attr_get_charray(e, data->value);
+	/* Pull out the name of the attribute which holds the key. */
+	key = slapi_entry_attr_get_charptr(e, data->key_attr);
+	/* Pull out the value format string and generate the value. XXX */
+	value = slapi_entry_attr_get_charptr(e, data->value_fmt);
+	/* Pull together the DN of this entry and the others we peeked at. */
 	dn = slapi_entry_get_dn(e);
-	for (i = 0; (key != NULL) && (key[i] != NULL); i++) {
-		for (j = 0; (value != NULL) && (value[j] != NULL); j++) {
-			slapi_log_error(SLAPI_LOG_PLUGIN,
-					data->state->plugin_desc->spd_id,
-					"setting domain/map/key/value "
-					"\"%s\"/\"%s\"/\"%s\"(\"%s\")=\"%s\"\n",
-					data->domain, data->map,
-					key[i], dn, value[j]);
-			map_data_set_entry(data->state, data->domain, data->map,
-					   dn, NULL,
-					   -1, strdup(key[i]),
-					   -1, strdup(value[j]));
-		}
-		if (j == 0) {
-			slapi_log_error(SLAPI_LOG_PLUGIN,
-					data->state->plugin_desc->spd_id,
-					"no value for %s\n", dn);
-		}
-	}
-	if (i == 0) {
+	visited_dn = NULL;
+	/* If we actually generated a value, then set it for all keys. */
+	if ((key != NULL) && (value != NULL)) {
+		/* For each key, set the value. */
 		slapi_log_error(SLAPI_LOG_PLUGIN,
 				data->state->plugin_desc->spd_id,
-				"no key for %s\n", dn);
+				"setting domain/map/key/value "
+				"\"%s\"/\"%s\"/\"%s\"(\"%s\")=\"%s\"\n",
+				data->domain, data->map,
+				key, dn, value);
+		map_data_set_entry(data->state, data->domain, data->map,
+				   dn, (const char **) visited_dn,
+				   -1, key,
+				   -1, value);
+	} else {
+		slapi_log_error(SLAPI_LOG_PLUGIN,
+				data->state->plugin_desc->spd_id,
+				"no value for %s\n", dn);
+		slapi_log_error(SLAPI_LOG_PLUGIN,
+				data->state->plugin_desc->spd_id,
+				"unsetting domain/map/id"
+				"\"%s\"/\"%s\"/(\"%s\")\n",
+				data->domain, data->map, dn);
+		map_data_unset_entry_id(data->state,
+					data->domain, data->map,
+					dn);
 	}
-	slapi_ch_array_free(value);
-	slapi_ch_array_free(key);
+	slapi_ch_free_string(&value);
+	slapi_ch_free_string(&key);
 	return 0;
 }
 
+/*
+ * Generate a copy of the filter string, with specific sequences replaced:
+ * %d -> name of the domain
+ * %m -> name of the map
+ * %% -> literal '%'
+ */
 static char *
 backend_map_config_filter(const char *fmt, const char *domain, const char *map)
 {
@@ -121,48 +176,62 @@ backend_map_config_filter(const char *fmt, const char *domain, const char *map)
 	return ret;
 }
 
+/* Given a directory server entry which represents a map's configuration, set
+ * up and populate the map. */
 static int
 backend_map_config_entry(struct plugin_state *state, Slapi_Entry *e,
 			 const char *domain, const char *map)
 {
 	Slapi_PBlock *pb;
 	char **base;
-	int i, scope;
+	int i;
 	char *entry_filter, *use_entry_filter, *key_attribute, *value_attribute;
-	const char *use_key_attribute, *use_value_attribute;
-	char *attrs[3]; /* XXX */
+	char *use_key_attribute, *use_value_attribute;
+	char *use_attrs[3]; /* XXX */
 	struct backend_map_entry_cb_data cb_data;
 
 	pb = slapi_pblock_new();
+	/* Read the NIS entry search base, entry search filter, and the names
+	 * of the key and value attributes. */
 	base = slapi_entry_attr_get_charray(e, "base");
 	entry_filter = slapi_entry_attr_get_charptr(e, "filter");
 	key_attribute = slapi_entry_attr_get_charptr(e, "key");
 	value_attribute = slapi_entry_attr_get_charptr(e, "value");
+	/* Perform substitutions on the filter, so that it can include the map
+	 * name without having to explicitly list it.  We need this because
+	 * RFC2307bis actually stores the map name in each entry, so it's
+	 * useful to be able to filter on it. */
 	use_entry_filter = backend_map_config_filter(entry_filter ?
 						     entry_filter :
 						     DEFAULT_ENTRY_FILTER,
 						     domain, map);
+	/* If the key and value attributes aren't set, we'll use compiled-in
+	 * defaults. */
 	use_key_attribute = key_attribute ?
 			    key_attribute :
 			    DEFAULT_KEY_ATTRIBUTE;
 	use_value_attribute = value_attribute ?
 			      value_attribute :
 			      DEFAULT_VALUE_ATTRIBUTE;
-	attrs[0] = (char *) use_key_attribute; /* XXX */
-	attrs[1] = (char *) use_value_attribute; /* XXX */
-	attrs[2] = NULL; /* XXX */
-	scope = LDAP_SCOPE_SUB;
+	/* Now get ready to search for entries which need to be in the map. */
+	use_attrs[0] = use_key_attribute; /* XXX */
+	use_attrs[1] = use_value_attribute; /* XXX */
+	use_attrs[2] = NULL; /* XXX */
 	cb_data.state = state;
-	cb_data.domain = domain;
-	cb_data.map = map;
-	cb_data.key = use_key_attribute;
-	cb_data.value = use_value_attribute;
+	cb_data.domain = (char *) domain;
+	cb_data.map = (char *) map;
+	cb_data.key_attr = use_key_attribute;
+	cb_data.value_fmt = use_value_attribute;
 	slapi_log_error(SLAPI_LOG_PLUGIN,
 			state->plugin_desc->spd_id,
 			"initializing map %s in %s (2)\n",
 			map, domain);
-	map_data_set_map(state, domain, map, NULL, NULL);
+	map_data_set_map(state, domain, map,
+			 backend_copy_cb_data(&cb_data),
+			 &backend_free_cb_data);
 	map_data_clear_map(state, domain, map);
+	/* Search under each base in turn, adding the matching directory
+	 * entries to the NIS maps. */
 	for (i = 0; (base != NULL) && (base[i] != NULL); i++) {
 		slapi_log_error(SLAPI_LOG_PLUGIN,
 				state->plugin_desc->spd_id,
@@ -170,9 +239,9 @@ backend_map_config_entry(struct plugin_state *state, Slapi_Entry *e,
 				base[i], use_entry_filter);
 		slapi_search_internal_set_pb(pb,
 					     base[i],
-					     scope,
+					     LDAP_SCOPE_SUB,
 					     use_entry_filter,
-					     attrs, 0,
+					     use_attrs, 0,
 					     NULL,
 					     NULL,
 					     state->plugin_identity,
@@ -183,6 +252,7 @@ backend_map_config_entry(struct plugin_state *state, Slapi_Entry *e,
 						  NULL);
 		slapi_free_search_results_internal(pb);
 	}
+	/* Clean up. */
 	free(use_entry_filter);
 	slapi_ch_free_string(&value_attribute);
 	slapi_ch_free_string(&key_attribute);
@@ -192,6 +262,9 @@ backend_map_config_entry(struct plugin_state *state, Slapi_Entry *e,
 	return 0;
 }
 
+/* Process a map configuration directory entry.  Pull out the domain and map
+ * names which are valid for this configuration and configure such a map for
+ * each in turn. */
 static int
 backend_map_config_entry_cb(Slapi_Entry *e, void *callback_data)
 {
@@ -246,16 +319,36 @@ backend_startup(struct plugin_state *state)
 }
 
 /* Our postop callbacks. */
+
+/* Add any map entries which correspond to a directory server entry in this
+ * map. */
+static PRBool
+backend_add_entry_cb(const char *domain, const char *map, void *backend_data,
+		     void *cbdata)
+{
+	char *dn;
+	struct backend_map_entry_cb_data *data;
+	data = backend_data;
+	dn = cbdata;
+	return PR_TRUE;
+}
+
 int
 backend_add_cb(Slapi_PBlock *pb)
 {
 	struct plugin_state *state;
-	char *dn, *id;
+	char *dn;
 	slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &state);
 	slapi_pblock_get(pb, SLAPI_TARGET_DN, &dn);
-	slapi_pblock_get(pb, SLAPI_TARGET_UNIQUEID, &id);
 	slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,
-			"added \"%s\" (\"%s\")\n", dn, id);
+			"added \"%s\"\n", dn);
+	/* Add map entries which corresponded to this directory server
+	 * entry. */
+	if (!map_data_foreach_map(state, NULL, backend_add_entry_cb, dn)) {
+		slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,
+				"error adding map entries corresponding to "
+				"\"%s\"\n", dn);
+	}
 	return 0;
 }
 
@@ -285,16 +378,38 @@ backend_modrdn_cb(Slapi_PBlock *pb)
 	return 0;
 }
 
+/* Delete any map entries which correspond to a directory server entry in this
+ * map. */
+static PRBool
+backend_delete_entry_cb(const char *domain, const char *map, void *backend_data,
+			void *cbdata)
+{
+	char *dn;
+	struct backend_map_entry_cb_data *data;
+	data = backend_data;
+	dn = cbdata;
+	map_data_unset_entry_id(data->state, domain, map, dn);
+	return PR_TRUE;
+}
+
+/* Called by the server when a directory server entry is deleted. */
 int
 backend_delete_cb(Slapi_PBlock *pb)
 {
 	struct plugin_state *state;
-	char *dn, *id;
+	char *dn;
 	slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &state);
 	slapi_pblock_get(pb, SLAPI_TARGET_DN, &dn);
-	slapi_pblock_get(pb, SLAPI_TARGET_UNIQUEID, &id);
 	slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,
-			"deleted \"%s\" (\"%s\")\n", dn, id);
+			"deleted \"%s\" (\"%s\")\n", dn);
+	/* Remove map entries which corresponded to this directory server
+	 * entry. */
+	if (!map_data_foreach_map(state, NULL, backend_delete_entry_cb, dn)) {
+		slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,
+				"error removing map entries corresponding to "
+				"\"%s\"\n", dn);
+	}
+	/* If it's a map configuration entry, remove the map. XXX */
 	return 0;
 }
 
@@ -325,3 +440,15 @@ backend_init(Slapi_PBlock *pb, struct plugin_state *state)
 				"error hooking up delete callback\n");
 	}
 }
+
+void *
+xmemdup(char *region, int size)
+{
+	char *ret;
+	ret = malloc(size + 1);
+	if (ret != NULL) {
+		memcpy(ret, region, size);
+		ret[size] = '\0';
+	}
+	return ret;
+}
diff --git a/src/backend.h b/src/backend.h
index 6bc3099..ed6bcd5 100644
--- a/src/backend.h
+++ b/src/backend.h
@@ -3,3 +3,4 @@ struct plugin_state;
 struct slapi_pblock;
 void backend_startup(struct plugin_state *state);
 void backend_init(struct slapi_pblock *pb, struct plugin_state *state);
+void *xmemdup(char *region, int size);
diff --git a/src/map.c b/src/map.c
index 387dfbf..13155aa 100644
--- a/src/map.c
+++ b/src/map.c
@@ -19,6 +19,7 @@
 #include "map.h"
 #include "portmap.h"
 
+/* The singleton for the cache. */
 struct {
 	struct domain {
 		char *name;
@@ -27,7 +28,7 @@ struct {
 			time_t last_changed;
 			struct entry {
 				struct entry *prev, *next;
-				char *id, **visited_ids;
+				char *id, **related_ids;
 				char *key;
 				unsigned int key_len;
 				char *value;
@@ -121,6 +122,8 @@ map_data_find_map_entry_id(struct plugin_state *state,
 	return NULL;
 }
 
+/* Handlers for the list of related IDs.  On this side we actually store them
+ * as one contiguous chunk of memory. */
 static PRBool
 map_id_in_id_list(const char *id, char **id_list)
 {
@@ -133,39 +136,60 @@ map_id_in_id_list(const char *id, char **id_list)
 	return FALSE;
 }
 
+/* Duplicate a passed-in list of strings.  The result is stored in one chunk of
+ * memory: pointers first, then the individual strings. */
 static char **
 map_id_dup_id_list(const char **in_id_list)
 {
 	int i, l;
 	char **ret, *s;
+	/* Handle the NULL case. */
+	if (in_id_list == NULL) {
+		return NULL;
+	}
+	/* Count the amount of space needed for the strings. */
 	for (i = 0, l = 0; (in_id_list[i] != NULL); i++) {
 		l += (strlen(in_id_list[i]) + 1);
 	}
+	/* No strings = no list. */
 	if (i == 0) {
 		return NULL;
 	}
+	/* Allocate space for the array of pointers (with NULL terminator) and
+	 * then the string data. */
 	ret = malloc((i + 1) * sizeof(char *) + l);
 	if (ret != NULL) {
+		/* Figure out where the string data will start. */
 		s = (char *) &ret[i + 1];
 		for (i = 0; (in_id_list[i] != NULL); i++) {
+			/* Set the address of this string, copy the data
+			 * around, and then prepare the address of the next
+			 * string. */
 			ret[i] = s;
 			strcpy(s, in_id_list[i]);
 			s += (strlen(s) + 1);
 		}
+		/* NULL-terminate the array. */
 		ret[i] = NULL;
 	}
 	return ret;
 }
 
+/* Free the list -- since it's one chunk of memory, we shouldn't try anything
+ * complicated. */
 static void
 map_id_free_id_list(char **id_list)
 {
 	free(id_list);
 }
 
+/* Iterate through every entry in every map, calling the callback if "all" is
+ * true, or if "id" is not NULL and matches the entry's ID, or if the
+ * "related_id" is not NULL and in the list of related IDs.  If the callback
+ * returns FALSE, then we abort and return FALSE, otherwise we return TRUE. */
 static PRBool
 map_data_foreach_entry(struct plugin_state *state,
-		       PRBool all, const char *id, const char *visited_id,
+		       PRBool all, const char *id, const char *related_id,
 		       PRBool (*fn)(const char *domain, const char *map,
 				    const char *key, unsigned int key_len,
 				    const char *value, unsigned int value_len,
@@ -186,9 +210,9 @@ map_data_foreach_entry(struct plugin_state *state,
 				if (all ||
 				    ((id != NULL) &&
 				     (strcmp(id, entry->id) == 0)) ||
-				    ((visited_id != NULL) &&
-				     (map_id_in_id_list(visited_id,
-						        entry->visited_ids)))) {
+				    ((related_id != NULL) &&
+				     (map_id_in_id_list(related_id,
+						        entry->related_ids)))) {
 					if (!(*fn)(domain->name, map->name,
 						   entry->key,
 						   entry->key_len,
@@ -204,6 +228,7 @@ map_data_foreach_entry(struct plugin_state *state,
 	return TRUE;
 }
 
+/* Iterate over every entry which matches the corresponding ID. */
 PRBool
 map_data_foreach_entry_id(struct plugin_state *state, const char *id,
 			  PRBool (*fn)(const char *domain, const char *map,
@@ -217,9 +242,10 @@ map_data_foreach_entry_id(struct plugin_state *state, const char *id,
 	return map_data_foreach_entry(state, FALSE, id, NULL, fn, cbdata);
 }
 
+/* Iterate over every entry which lists the ID as a related ID. */
 PRBool
-map_data_foreach_entry_visited_id(struct plugin_state *state,
-				  const char *visited_id,
+map_data_foreach_entry_related_id(struct plugin_state *state,
+				  const char *related_id,
 				  PRBool (*fn)(const char *domain,
 					       const char *map,
 					       const char *key,
@@ -229,10 +255,42 @@ map_data_foreach_entry_visited_id(struct plugin_state *state,
 					       const char *id, void *cbdata),
 				  void *cbdata)
 {
-	return map_data_foreach_entry(state, FALSE, NULL, visited_id,
+	return map_data_foreach_entry(state, FALSE, NULL, related_id,
 				      fn, cbdata);
 }
 
+/* Iterate over all maps, calling the callback with information about the map
+ * and whatever the caller originally told us to keep track of when the map was
+ * first set up. */
+PRBool
+map_data_foreach_map(struct plugin_state *state, const char *domain_name,
+		     PRBool (*fn)(const char *domain,
+		    		  const char *map,
+				  void *backend_data,
+				  void *cbdata),
+		     void *cbdata)
+{
+	int i, j;
+	struct domain *domain;
+	struct map *map;
+	struct entry *entry;
+	for (i = 0; i < map_data.n_domains; i++) {
+		domain = &map_data.domains[i];
+		if ((domain_name != NULL) &&
+		    (strcmp(domain->name, domain_name) != 0)) {
+			continue;
+		}
+		for (j = 0; j < domain->n_maps; j++) {
+			map = &domain->maps[j];
+			if (!(*fn)(domain->name, map->name, map->backend_data,
+				   cbdata)) {
+				return FALSE;
+			}
+		}
+	}
+	return TRUE;
+}
+
 /* Query function: check if we have a record for the domain.  Return that
  * information in "supported", and return TRUE unless we ran into internal
  * errors. */
@@ -504,6 +562,8 @@ map_data_set_map(struct plugin_state *state,
 			}
 		}
 	} else {
+		/* There's already a map there, we just need to update the
+		 * data we're keeping track of for the backend. */
 		if (map->free_backend_data != NULL) {
 			map->free_backend_data(map->backend_data);
 		}
@@ -531,6 +591,7 @@ map_data_unset_map_entry(struct plugin_state *state,
 		if (map->entries == entry) {
 			map->entries = next;
 		}
+		map_id_free_id_list(entry->related_ids);
 		free(entry->id);
 		free(entry->key);
 		free(entry->value);
@@ -538,6 +599,7 @@ map_data_unset_map_entry(struct plugin_state *state,
 	}
 }
 
+/* Remove the entry from the map which matches the passed-in key. */
 void
 map_data_unset_entry_key(struct plugin_state *state,
 			 const char *domain_name,
@@ -553,6 +615,7 @@ map_data_unset_entry_key(struct plugin_state *state,
 	map->last_changed = time(NULL);
 }
 
+/* Remove the entry from the map which matches the passed-in ID. */
 void
 map_data_unset_entry_id(struct plugin_state *state,
 			const char *domain_name,
@@ -573,7 +636,7 @@ map_data_set_entry(struct plugin_state *state,
 		   const char *domain_name,
 		   const char *map_name,
 		   const char *id,
-		   const char **visited_ids,
+		   const char **related_ids,
 		   unsigned int key_len,
 		   char *key,
 		   unsigned int value_len,
@@ -591,26 +654,29 @@ map_data_set_entry(struct plugin_state *state,
 	if (map != NULL) {
 		entry = map_data_find_map_entry_id(state, map, id);
 		if (entry != NULL) {
+			/* There's already an entry with this ID, so let's
+			 * replace its key and value. */
 			free(entry->key);
-			free(entry->value);
-			entry->key = key;
+			entry->key = xmemdup(key, key_len);
 			entry->key_len = key_len;
-			entry->value = value;
+			free(entry->value);
+			entry->value = xmemdup(value, value_len);
 			entry->value_len = value_len;
 			free(entry->id);
 			entry->id = strdup(id);
-			map_id_free_id_list(entry->visited_ids);
-			entry->visited_ids = map_id_dup_id_list(visited_ids);
+			map_id_free_id_list(entry->related_ids);
+			entry->related_ids = map_id_dup_id_list(related_ids);
 		} else {
+			/* There's no entry with this ID, so create one. */
 			entry = malloc(sizeof(*entry));
 			if (entry != NULL) {
 				memset(entry, 0, sizeof(*entry));
-				entry->key = key;
+				entry->key = xmemdup(key, key_len);
 				entry->key_len = key_len;
-				entry->value = value;
+				entry->value = xmemdup(value, value_len);
 				entry->value_len = value_len;
 				entry->id = strdup(id);
-				entry->visited_ids = map_id_dup_id_list(visited_ids);
+				entry->related_ids = map_id_dup_id_list(related_ids);
 				entry->next = map->entries;
 				if (map->entries != NULL) {
 					map->entries->prev = entry;
diff --git a/src/map.h b/src/map.h
index 48ba5ae..cc479b0 100644
--- a/src/map.h
+++ b/src/map.h
@@ -42,7 +42,7 @@ void map_data_unset_entry_id(struct plugin_state *state,
 			     const char *id);
 void map_data_set_entry(struct plugin_state *state,
 			const char *domain_name, const char *map_name,
-			const char *id, const char **visited_ids,
+			const char *id, const char **related_ids,
 			unsigned int key_len, char *key,
 			unsigned int value_len, char *value);
 PRBool map_data_foreach_entry_id(struct plugin_state *state, const char *id,
@@ -54,8 +54,8 @@ PRBool map_data_foreach_entry_id(struct plugin_state *state, const char *id,
 					      unsigned int value_len,
 					      const char *id, void *cbdata),
 				 void *cbdata);
-PRBool map_data_foreach_entry_visited_id(struct plugin_state *state,
-					 const char *visited_id,
+PRBool map_data_foreach_entry_related_id(struct plugin_state *state,
+					 const char *related_id,
 					 PRBool (*fn)(const char *domain,
 						      const char *map,
 						      const char *key,
@@ -65,3 +65,9 @@ PRBool map_data_foreach_entry_visited_id(struct plugin_state *state,
 						      const char *id,
 						      void *cbdata),
 					 void *cbdata);
+PRBool map_data_foreach_map(struct plugin_state *state, const char *domain_name,
+			    PRBool (*fn)(const char *domain,
+			   		 const char *map,
+					 void *backend_data,
+					 void *cbdata),
+			    void *cbdata);