* 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

diff --git a/src/util/profile/ChangeLog b/src/util/profile/ChangeLog
index 4c68f89c0d3595908775deb7ba94ea0e431b0e05..6a8bfa4247958a95d6fbfd1cc904bf9cddd6db1a 100644 (file)
--- 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 8754328730b56771988c686566c3f62bffe23347..6220ca62f39e09c0564d7697051eca87818cfcf5 100644 (file)
--- 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 2062b7053f25b7903ed786847841b00b9d4a642f..4a1d7b65ade5fe2f0e3f24a370c4eede9046903f 100644 (file)
--- 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 3c44898606d952eb85576d08686d2d4553e69ff7..f1e3daf63931c4128a9edf4d707421695f133124 100644 (file)
--- 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 916620eda50a84522892c043145ed588705c2a26..cf7f33f9cf4bbf476b68bb8e479d872b1d24e098 100644 (file)
--- 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;