From c6bc3153efcd345d411695e449c1a1475f84655a Mon Sep 17 00:00:00 2001 From: David Sidrane Date: Fri, 15 May 2015 03:58:04 -1000 Subject: [PATCH] Reviewd - fixed indexing that was wrong, code clean up ran astyle --- src/modules/systemlib/param/param.c | 126 +++++++++++++++------------- 1 file changed, 70 insertions(+), 56 deletions(-) diff --git a/src/modules/systemlib/param/param.c b/src/modules/systemlib/param/param.c index e86f09ffae..6de03125ca 100644 --- a/src/modules/systemlib/param/param.c +++ b/src/modules/systemlib/param/param.c @@ -72,13 +72,13 @@ * Array of static parameter info. */ #ifdef _UNIT_TEST - extern struct param_info_s param_array[]; - extern struct param_info_s *param_info_base; - extern struct param_info_s *param_info_limit; +extern struct param_info_s param_array[]; +extern struct param_info_s *param_info_base; +extern struct param_info_s *param_info_limit; #else - extern char __param_start, __param_end; - static const struct param_info_s *param_info_base = (struct param_info_s *) &__param_start; - static const struct param_info_s *param_info_limit = (struct param_info_s *) &__param_end; +extern char __param_start, __param_end; +static const struct param_info_s *param_info_base = (struct param_info_s *) &__param_start; +static const struct param_info_s *param_info_limit = (struct param_info_s *) &__param_end; #endif #define param_info_count ((unsigned)(param_info_limit - param_info_base)) @@ -101,13 +101,13 @@ const int bits_per_allocation_unit = (sizeof(*param_changed_storage) * 8); static unsigned get_param_info_count(void) { - // Singleton - if (!param_changed_storage) - { - size_param_changed_storage_bytes = (param_info_count / bits_per_allocation_unit ) + 1; - param_changed_storage = calloc(size_param_changed_storage_bytes, 1); - } - return param_info_count; + /* Singleton creation of and array of bits to track changed values */ + if (!param_changed_storage) { + size_param_changed_storage_bytes = (param_info_count / bits_per_allocation_unit) + 1; + param_changed_storage = calloc(size_param_changed_storage_bytes, 1); + } + + return param_info_count; } /** flexible array holding modified parameter values */ @@ -170,11 +170,13 @@ param_compare_values(const void *a, const void *b) struct param_wbuf_s *pa = (struct param_wbuf_s *)a; struct param_wbuf_s *pb = (struct param_wbuf_s *)b; - if (pa->param < pb->param) + if (pa->param < pb->param) { return -1; + } - if (pa->param > pb->param) + if (pa->param > pb->param) { return 1; + } return 0; } @@ -187,7 +189,8 @@ param_compare_values(const void *a, const void *b) * NULL if the parameter has not been modified. */ static struct param_wbuf_s * -param_find_changed(param_t param) { +param_find_changed(param_t param) +{ struct param_wbuf_s *s = NULL; param_assert_locked(); @@ -200,8 +203,9 @@ param_find_changed(param_t param) { #else while ((s = (struct param_wbuf_s *)utarray_next(param_values, s)) != NULL) { - if (s->param == param) + if (s->param == param) { break; + } } #endif @@ -238,6 +242,7 @@ param_find_internal(const char *name, bool notification) if (notification) { param_set_used_internal(param); } + return param; } } @@ -267,9 +272,10 @@ param_count(void) unsigned param_count_used(void) { - // ensure the allocation has been done - get_param_info_count(); + // ensure the allocation has been done + get_param_info_count(); unsigned count = 0; + for (unsigned i = 0; i < size_param_changed_storage_bytes; i++) { for (unsigned j = 0; j < bits_per_allocation_unit; j++) { if (param_changed_storage[i] & (1 << j)) { @@ -277,6 +283,7 @@ param_count_used(void) } } } + return count; } @@ -359,10 +366,7 @@ param_get_used_index(param_t param) const char * param_name(param_t param) { - if (handle_in_range(param)) - return param_info_base[param].name; - - return NULL; + return handle_in_range(param) ? param_info_base[param].name : NULL;; } bool @@ -375,29 +379,22 @@ bool param_value_unsaved(param_t param) { static struct param_wbuf_s *s; - s = param_find_changed(param); - - if (s && s->unsaved) - return true; - - return false; + return (s && s->unsaved) ? true : false; } enum param_type_e -param_type(param_t param) -{ - if (handle_in_range(param)) - return param_info_base[param].type; - - return PARAM_TYPE_UNKNOWN; +param_type(param_t param) { + return handle_in_range(param) ? param_info_base[param].type : PARAM_TYPE_UNKNOWN; } size_t param_size(param_t param) { if (handle_in_range(param)) { + switch (param_type(param)) { + case PARAM_TYPE_INT32: case PARAM_TYPE_FLOAT: return 4; @@ -442,8 +439,9 @@ param_get_value_ptr(param_t param) v = ¶m_info_base[param].val; } - if (param_type(param) >= PARAM_TYPE_STRUCT - && param_type(param) <= PARAM_TYPE_STRUCT_MAX) { + if (param_type(param) >= PARAM_TYPE_STRUCT && + param_type(param) <= PARAM_TYPE_STRUCT_MAX) { + result = v->p; } else { @@ -481,8 +479,9 @@ param_set_internal(param_t param, const void *val, bool mark_saved, bool notify_ param_lock(); - if (param_values == NULL) + if (param_values == NULL) { utarray_new(param_values, ¶m_icd); + } if (param_values == NULL) { debug("failed to allocate modified values array"); @@ -512,6 +511,7 @@ param_set_internal(param_t param, const void *val, bool mark_saved, bool notify_ /* update the changed value */ switch (param_type(param)) { + case PARAM_TYPE_INT32: s->val.i = *(int32_t *)val; break; @@ -551,8 +551,9 @@ out: * If we set something, now that we have unlocked, go ahead and advertise that * a thing has been set. */ - if (params_changed && notify_changes) + if (params_changed && notify_changes) { param_notify_changes(); + } return result; } @@ -573,23 +574,25 @@ bool param_used(param_t param) { int param_index = param_get_index(param); + if (param_index < 0) { return false; } - unsigned bitindex = param_index - (param_index / bits_per_allocation_unit); - return param_changed_storage[param_index / bits_per_allocation_unit] & (1 << bitindex); + return param_changed_storage[param_index / bits_per_allocation_unit] & + (1 << param_index % bits_per_allocation_unit); } void param_set_used_internal(param_t param) { int param_index = param_get_index(param); + if (param_index < 0) { return; } - unsigned bitindex = param_index - (param_index / bits_per_allocation_unit); - param_changed_storage[param_index / bits_per_allocation_unit] |= (1 << bitindex); + param_changed_storage[param_index / bits_per_allocation_unit] |= + (1 << param_index % bits_per_allocation_unit); } int @@ -616,8 +619,9 @@ param_reset(param_t param) param_unlock(); - if (s != NULL) + if (s != NULL) { param_notify_changes(); + } return (!param_found); } @@ -640,28 +644,28 @@ param_reset_all(void) } void -param_reset_excludes(const char* excludes[], int num_excludes) +param_reset_excludes(const char *excludes[], int num_excludes) { param_lock(); param_t param; for (param = 0; handle_in_range(param); param++) { - const char* name = param_name(param); + const char *name = param_name(param); bool exclude = false; for (int index = 0; index < num_excludes; index ++) { int len = strlen(excludes[index]); - if((excludes[index][len - 1] == '*' - && strncmp(name, excludes[index], len - 1) == 0) - || strcmp(name, excludes[index]) == 0) { + if ((excludes[index][len - 1] == '*' + && strncmp(name, excludes[index], len - 1) == 0) + || strcmp(name, excludes[index]) == 0) { exclude = true; break; } } - if(!exclude) { + if (!exclude) { param_reset(param); } } @@ -675,14 +679,17 @@ static const char *param_default_file = "/eeprom/parameters"; static char *param_user_file = NULL; int -param_set_default_file(const char* filename) +param_set_default_file(const char *filename) { if (param_user_file != NULL) { free(param_user_file); param_user_file = NULL; } - if (filename) + + if (filename) { param_user_file = strdup(filename); + } + return 0; } @@ -733,6 +740,7 @@ param_load_default(void) warn("open '%s' for reading failed", param_get_default_file()); return -1; } + return 1; } @@ -773,13 +781,16 @@ param_export(int fd, bool only_unsaved) * If we are only saving values changed since last save, and this * one hasn't, then skip it */ - if (only_unsaved && !s->unsaved) + if (only_unsaved && !s->unsaved) { continue; + } s->unsaved = false; /* append the appropriate BSON type object */ + switch (param_type(s->param)) { + case PARAM_TYPE_INT32: param_get(s->param, &i); @@ -823,8 +834,9 @@ param_export(int fd, bool only_unsaved) out: param_unlock(); - if (result == 0) + if (result == 0) { result = bson_encoder_fini(&encoder); + } return result; } @@ -934,8 +946,9 @@ param_import_callback(bson_decoder_t decoder, void *private, bson_node_t node) out: - if (tmp != NULL) + if (tmp != NULL) { free(tmp); + } return result; } @@ -961,8 +974,9 @@ param_import_internal(int fd, bool mark_saved) out: - if (result < 0) + if (result < 0) { debug("BSON error decoding parameters"); + } return result; }