* prof_int.h (struct _prf_data_t): Add a mutex.

* prof_file.c (profile_open_file): Initialize data mutex.
(profile_update_file_data, profile_flush_file_data): Lock it while manipulating
file data.
(profile_lock_global, profile_unlock_global): New functions.
* prof_set.c (rw_setup): Acquire global lock while checking flags and adjusting
ref count.
(profile_update_relation, profile_rename_section, profile_add_relation): Lock
data mutex while manipulating profile data.
* prof_tree.c (profile_node_iterator): Do more magic number tests.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@16689 dc483132-0cff-0310-8789-dd5450dbe970

5 files changed, 142 insertions, 50 deletions

diff --git a/src/util/profile/ChangeLog b/src/util/profile/ChangeLog
index 4c68f89..6a8bfa4 100644
--- a/src/util/profile/ChangeLog
+++ b/src/util/profile/ChangeLog
@@ -1,3 +1,18 @@
+2004-08-27  Ken Raeburn  <raeburn@mit.edu>
+
+	* prof_int.h (struct _prf_data_t): Add a mutex.
+	* prof_file.c (profile_open_file): Initialize data mutex.
+	(profile_update_file_data, profile_flush_file_data): Lock it while
+	manipulating file data.
+	(profile_lock_global, profile_unlock_global): New functions.
+	* prof_set.c (rw_setup): Acquire global lock while checking flags
+	and adjusting ref count.
+	(profile_update_relation, profile_rename_section,
+	profile_add_relation): Lock data mutex while manipulating profile
+	data.
+	* prof_tree.c (profile_node_iterator): Do more magic number
+	tests.
+
 2004-07-14  Ken Raeburn  <raeburn@mit.edu>
 
 	* libprofile.exports: Don't try to export
diff --git a/src/util/profile/prof_file.c b/src/util/profile/prof_file.c
index 8754328..6220ca6 100644
--- a/src/util/profile/prof_file.c
+++ b/src/util/profile/prof_file.c
@@ -217,8 +217,15 @@ errcode_t profile_open_file(const_profile_filespec_t filespec,
 	data->comment = 0;
 	data->filespec = expanded_filename;
 
+	retval = k5_mutex_init(&data->lock);
+	if (retval) {
+	    profile_close_file(prf);
+	    return retval;
+	}
+
 	retval = profile_update_file(prf);
 	if (retval) {
+		k5_mutex_destroy(&data->lock);
 		profile_close_file(prf);
 		return retval;
 	}
@@ -247,11 +254,20 @@ errcode_t profile_update_file_data(prf_data_t data)
 #endif
 	FILE *f;
 
+	retval = k5_mutex_lock(&data->lock);
+	if (retval)
+	    return retval;
+
 #ifdef HAVE_STAT
-	if (stat(data->filespec, &st))
-		return errno;
-	if (st.st_mtime == data->timestamp)
-		return 0;
+	if (stat(data->filespec, &st)) {
+	    retval = errno;
+	    k5_mutex_unlock(&data->lock);
+	    return retval;
+	}
+	if (st.st_mtime == data->timestamp) {
+	    k5_mutex_unlock(&data->lock);
+	    return 0;
+	}
 	if (data->root) {
 		profile_free_node(data->root);
 		data->root = 0;
@@ -266,13 +282,16 @@ errcode_t profile_update_file_data(prf_data_t data)
 	 * memory image is correct.  That is, we won't reread the
 	 * profile file if it changes.
 	 */
-	if (data->root)
-		return 0;
+	if (data->root) {
+	    k5_mutex_unlock(&data->lock);
+	    return 0;
+	}
 #endif
 	errno = 0;
 	f = fopen(data->filespec, "r");
 	if (f == NULL) {
 		retval = errno;
+		k5_mutex_unlock(&data->lock);
 		if (retval == 0)
 			retval = ENOENT;
 		return retval;
@@ -283,11 +302,14 @@ errcode_t profile_update_file_data(prf_data_t data)
 		data->flags |= PROFILE_FILE_RW;
 	retval = profile_parse_file(f, &data->root);
 	fclose(f);
-	if (retval)
-		return retval;
+	if (retval) {
+	    k5_mutex_unlock(&data->lock);
+	    return retval;
+	}
 #ifdef HAVE_STAT
 	data->timestamp = st.st_mtime;
 #endif
+	k5_mutex_unlock(&data->lock);
 	return 0;
 }
 
@@ -307,12 +329,18 @@ errcode_t profile_flush_file_data(prf_data_t data)
 	profile_filespec_t new_file;
 	profile_filespec_t old_file;
 	errcode_t	retval = 0;
-	
+
 	if (!data || data->magic != PROF_MAGIC_FILE_DATA)
 		return PROF_MAGIC_FILE_DATA;
+
+	retval = k5_mutex_lock(&data->lock);
+	if (retval)
+	    return retval;
 	
-	if ((data->flags & PROFILE_FILE_DIRTY) == 0)
-		return 0;
+	if ((data->flags & PROFILE_FILE_DIRTY) == 0) {
+	    k5_mutex_unlock(&data->lock);
+	    return 0;
+	}
 
 	retval = ENOMEM;
 	
@@ -379,6 +407,7 @@ errcode_t profile_flush_file_data(prf_data_t data)
 	retval = 0;
 	
 errout:
+	k5_mutex_unlock(&data->lock);
 	if (new_file)
 		free(new_file);
 	if (old_file)
@@ -403,6 +432,15 @@ void profile_dereference_data(prf_data_t data)
 #endif
 }
 
+int profile_lock_global()
+{
+    return k5_mutex_lock(&g_shared_trees_mutex);
+}
+int profile_unlock_global()
+{
+    return k5_mutex_unlock(&g_shared_trees_mutex);
+}
+
 void profile_free_file(prf_file_t prf)
 {
     profile_dereference_data(prf->data);
diff --git a/src/util/profile/prof_int.h b/src/util/profile/prof_int.h
index 2062b70..4a1d7b6 100644
--- a/src/util/profile/prof_int.h
+++ b/src/util/profile/prof_int.h
@@ -10,6 +10,8 @@
 #define PROFILE_SUPPORTS_FOREIGN_NEWLINES
 #endif
 
+/* N.B.: The locking code for the non-sharing case is not complete.
+   Be sure to fix it if you turn off this flag.  */
 #define SHARE_TREE_DATA
 
 #include "k5-thread.h"
@@ -27,9 +29,15 @@ typedef long prf_magic_t;
 /*
  * This is the structure which stores the profile information for a
  * particular configuration file.
+ *
+ * Locking strategy:
+ * - filespec is fixed after creation
+ * - refcount and next should only be tweaked with the global lock held
+ * - other fields can be tweaked after grabbing the in-struct lock
  */
 struct _prf_data_t {
 	prf_magic_t	magic;
+	k5_mutex_t	lock;
 	char		*comment;
 	profile_filespec_t filespec;
 	struct profile_node *root;
diff --git a/src/util/profile/prof_set.c b/src/util/profile/prof_set.c
index 3c44898..f1e3daf 100644
--- a/src/util/profile/prof_set.c
+++ b/src/util/profile/prof_set.c
@@ -32,12 +32,19 @@ static errcode_t rw_setup(profile_t profile)
 		return PROF_MAGIC_PROFILE;
 
 	file = profile->first_file;
+
 	if (!(file->data->flags & PROFILE_FILE_RW))
-		return PROF_READ_ONLY;
+	    return PROF_READ_ONLY;
+
+	retval = profile_lock_global();
+	if (retval)
+	    return retval;
 
 	/* Don't update the file if we've already made modifications */
-	if (file->data->flags & PROFILE_FILE_DIRTY)
-		return 0;
+	if (file->data->flags & PROFILE_FILE_DIRTY) {
+	    profile_unlock_global();
+	    return 0;
+	}
 
 #ifdef SHARE_TREE_DATA
 	if ((file->data->flags & PROFILE_FILE_SHARED) != 0) {
@@ -63,6 +70,7 @@ static errcode_t rw_setup(profile_t profile)
 	    }
 
 	    if (retval != 0) {
+		profile_unlock_global();
 		free(new_data);
 		return retval;
 	    }
@@ -71,6 +79,7 @@ static errcode_t rw_setup(profile_t profile)
 	}
 #endif /* SHARE_TREE_DATA */
 
+	profile_unlock_global();
 	retval = profile_update_file(file);
 	
 	return retval;
@@ -101,30 +110,33 @@ profile_update_relation(profile_t profile, const char **names,
 	if (!old_value || !*old_value)
 		return PROF_EINVAL;
 
+	retval = k5_mutex_lock(&profile->first_file->data->lock);
+	if (retval)
+	    return retval;
 	section = profile->first_file->data->root;
 	for (cpp = names; cpp[1]; cpp++) {
 		state = 0;
 		retval = profile_find_node(section, *cpp, 0, 1,
 					   &state, &section);
-		if (retval)
-			return retval;
+		if (retval) {
+		    k5_mutex_unlock(&profile->first_file->data->lock);
+		    return retval;
+		}
 	}
 
 	state = 0;
 	retval = profile_find_node(section, *cpp, old_value, 0, &state, &node);
-	if (retval)
-		return retval;
-
-	if (new_value)
+	if (retval == 0) {
+	    if (new_value)
 		retval = profile_set_relation_value(node, new_value);
-	else
+	    else
 		retval = profile_remove_node(node);
-	if (retval)
-		return retval;
-
-	profile->first_file->data->flags |= PROFILE_FILE_DIRTY;
+	}
+	if (retval == 0)
+	    profile->first_file->data->flags |= PROFILE_FILE_DIRTY;
+	k5_mutex_unlock(&profile->first_file->data->lock);
 	
-	return 0;
+	return retval;
 }
 
 /* 
@@ -139,7 +151,7 @@ profile_clear_relation(profile_t profile, const char **names)
 	struct profile_node *section, *node;
 	void		*state;
 	const char	**cpp;
-	
+
 	retval = rw_setup(profile);
 	if (retval)
 		return retval;
@@ -193,30 +205,32 @@ profile_rename_section(profile_t profile, const char **names,
 	if (names == 0 || names[0] == 0 || names[1] == 0)
 		return PROF_BAD_NAMESET;
 
+	retval = k5_mutex_lock(&profile->first_file->data->lock);
+	if (retval)
+	    return retval;
 	section = profile->first_file->data->root;
 	for (cpp = names; cpp[1]; cpp++) {
 		state = 0;
 		retval = profile_find_node(section, *cpp, 0, 1,
 					   &state, &section);
-		if (retval)
-			return retval;
+		if (retval) {
+		    k5_mutex_unlock(&profile->first_file->data->lock);
+		    return retval;
+		}
 	}
 
 	state = 0;
 	retval = profile_find_node(section, *cpp, 0, 1, &state, &node);
-	if (retval)
-		return retval;
-
-	if (new_name)
+	if (retval == 0) {
+	    if (new_name)
 		retval = profile_rename_node(node, new_name);
-	else
+	    else
 		retval = profile_remove_node(node);
-	if (retval)
-		return retval;
-
-	profile->first_file->data->flags |= PROFILE_FILE_DIRTY;
-	
-	return 0;
+	}
+	if (retval == 0)
+	    profile->first_file->data->flags |= PROFILE_FILE_DIRTY;
+	k5_mutex_unlock(&profile->first_file->data->lock);
+	return retval;
 }
 
 /*
@@ -244,6 +258,9 @@ profile_add_relation(profile_t profile, const char **names,
 	if (names == 0 || names[0] == 0 || names[1] == 0)
 		return PROF_BAD_NAMESET;
 
+	retval = k5_mutex_lock(&profile->first_file->data->lock);
+	if (retval)
+	    return retval;
 	section = profile->first_file->data->root;
 	for (cpp = names; cpp[1]; cpp++) {
 		state = 0;
@@ -251,24 +268,31 @@ profile_add_relation(profile_t profile, const char **names,
 					   &state, &section);
 		if (retval == PROF_NO_SECTION)
 			retval = profile_add_node(section, *cpp, 0, &section);
-		if (retval)
-			return retval;
+		if (retval) {
+		    k5_mutex_unlock(&profile->first_file->data->lock);
+		    return retval;
+		}
 	}
 
 	if (new_value == 0) {
 		retval = profile_find_node(section, *cpp, 0, 1, &state, 0);
-		if (retval == 0)
-			return PROF_EXISTS;
-		else if (retval != PROF_NO_SECTION)
-			return retval;
+		if (retval == 0) {
+		    k5_mutex_unlock(&profile->first_file->data->lock);
+		    return PROF_EXISTS;
+		} else if (retval != PROF_NO_SECTION) {
+		    k5_mutex_unlock(&profile->first_file->data->lock);
+		    return retval;
+		}
 	}
 
 	retval = profile_add_node(section, *cpp, new_value, 0);
-	if (retval)
-		return retval;
+	if (retval) {
+	    k5_mutex_unlock(&profile->first_file->data->lock);
+	    return retval;
+	}
 
 	profile->first_file->data->flags |= PROFILE_FILE_DIRTY;
-	
+	k5_mutex_unlock(&profile->first_file->data->lock);
 	return 0;
 }
 
diff --git a/src/util/profile/prof_tree.c b/src/util/profile/prof_tree.c
index 916620e..cf7f33f 100644
--- a/src/util/profile/prof_tree.c
+++ b/src/util/profile/prof_tree.c
@@ -454,6 +454,10 @@ errcode_t profile_node_iterator(void **iter_p, struct profile_node **ret_node,
 
 	if (!iter || iter->magic != PROF_MAGIC_ITERATOR)
 		return PROF_MAGIC_ITERATOR;
+	if (iter->file && iter->file->magic != PROF_MAGIC_FILE)
+	    return PROF_MAGIC_FILE;
+	if (iter->file && iter->file->data->magic != PROF_MAGIC_FILE_DATA)
+	    return PROF_MAGIC_FILE_DATA;
 	/*
 	 * If the file has changed, then the node pointer is invalid,
 	 * so we'll have search the file again looking for it.
@@ -463,6 +467,8 @@ errcode_t profile_node_iterator(void **iter_p, struct profile_node **ret_node,
 		skip_num = iter->num;
 		iter->node = 0;
 	}
+	if (iter->node && iter->node->magic != PROF_MAGIC_NODE)
+	    return PROF_MAGIC_NODE;
 get_new_file:
 	if (iter->node == 0) {
 		if (iter->file == 0 ||
@@ -495,9 +501,10 @@ get_new_file:
 		 */
 		section = iter->file->data->root;
 		for (cpp = iter->names; cpp[iter->done_idx]; cpp++) {
-			for (p=section->first_child; p; p = p->next)
+			for (p=section->first_child; p; p = p->next) {
 				if (!strcmp(p->name, *cpp) && !p->value)
 					break;
+			}
 			if (!p) {
 				section = 0;
 				break;