From b06c64321b0cef679623d264f19ba64e625d1f32 Mon Sep 17 00:00:00 2001 From: Ken Raeburn Date: Fri, 27 Aug 2004 19:41:53 +0000 Subject: [PATCH] * 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 --- src/util/profile/ChangeLog | 15 ++++++ src/util/profile/prof_file.c | 60 +++++++++++++++++---- src/util/profile/prof_int.h | 8 +++ src/util/profile/prof_set.c | 100 ++++++++++++++++++++++------------- src/util/profile/prof_tree.c | 9 +++- 5 files changed, 142 insertions(+), 50 deletions(-) diff --git a/src/util/profile/ChangeLog b/src/util/profile/ChangeLog index 4c68f89c0..6a8bfa424 100644 --- a/src/util/profile/ChangeLog +++ b/src/util/profile/ChangeLog @@ -1,3 +1,18 @@ +2004-08-27 Ken Raeburn + + * 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 * libprofile.exports: Don't try to export diff --git a/src/util/profile/prof_file.c b/src/util/profile/prof_file.c index 875432873..6220ca62f 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 2062b7053..4a1d7b65a 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 3c4489860..f1e3daf63 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, §ion); - 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, §ion); - 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, §ion); if (retval == PROF_NO_SECTION) retval = profile_add_node(section, *cpp, 0, §ion); - 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 916620eda..cf7f33f9c 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; -- 2.26.2