From 1069fb127a38d875d0ca2f56bfc936a97a964380 Mon Sep 17 00:00:00 2001 From: Dmitri Pal Date: Fri, 14 Aug 2009 23:04:37 -0400 Subject: COMMON Fixes to return values, errno, leaks Started looking at the ticket #107 related to traverse functions. Realized that the return values are not consistent. That ovelapped with the work that I wanted to do for ticket #103 - errno cleanup. So I (across collection, INI and ELAPI): * Made the return codes consistent (where found) * Removed errno where it is not needed While was testing used valgrind and found a nasty problem when the value was added to collection with overwriting duplicates the count was decreased improperly. Fixing collection.c to not decrease count made valgrind happy. While I was debugging this I also spotted several build warnings in trace statements when the " exp ? v1 : v2 " was used. Fixed those. In ini_config.c there was a trace stament that used variable after it was freed. Removed trace stament. --- common/collection/collection.c | 71 ++++++++++++++++++++++-------------- common/collection/collection_queue.c | 4 +- common/collection/collection_stack.c | 4 +- common/collection/collection_tools.c | 24 +++++------- common/elapi/elapi_internal.c | 6 +-- common/elapi/elapi_log.c | 12 +++--- common/ini/ini_config.c | 10 ++--- 7 files changed, 69 insertions(+), 62 deletions(-) diff --git a/common/collection/collection.c b/common/collection/collection.c index d6fd872fd..8d3b3b5d5 100644 --- a/common/collection/collection.c +++ b/common/collection/collection.c @@ -187,8 +187,6 @@ static int col_allocate_item(struct collection_item **ci, const char *property, const void *item_data, int length, int type) { struct collection_item *item = NULL; - int error = 0; - errno = 0; TRACE_FLOW_STRING("col_allocate_item", "Entry point."); TRACE_INFO_NUMBER("Will be using type:", type); @@ -207,9 +205,8 @@ static int col_allocate_item(struct collection_item **ci, const char *property, /* Allocate memory for the structure */ item = (struct collection_item *)malloc(sizeof(struct collection_item)); if (item == NULL) { - error = errno; TRACE_ERROR_STRING("col_allocate_item", "Malloc failed."); - return error; + return ENOMEM; } /* After we initialize "next" we can use delete_item() in case of error */ @@ -218,10 +215,9 @@ static int col_allocate_item(struct collection_item **ci, const char *property, /* Copy property */ item->property = strdup(property); if (item->property == NULL) { - error = errno; TRACE_ERROR_STRING("col_allocate_item", "Failed to dup property."); col_delete_item(item); - return error; + return ENOMEM; } item->phash = FNV1a_base; @@ -242,7 +238,7 @@ static int col_allocate_item(struct collection_item **ci, const char *property, if (item->data == NULL) { TRACE_ERROR_STRING("col_allocate_item", "Failed to dup data."); col_delete_item(item); - return errno; + return ENOMEM; } memcpy(item->data, item_data, length); @@ -260,7 +256,7 @@ static int col_allocate_item(struct collection_item **ci, const char *property, TRACE_INFO_NUMBER("Item property type", item->type); TRACE_INFO_NUMBER("Item data length", item->length); TRACE_FLOW_STRING("col_allocate_item", "Success exit."); - return 0; + return EOK; } /* Structure used to find things in collection */ @@ -321,7 +317,7 @@ static int col_find_property(struct collection_item *collection, /* Item is not found */ TRACE_FLOW_STRING("col_find_property", "Exit - item NOT found"); - return 0; + return EOK; } @@ -387,7 +383,7 @@ int col_insert_item_into_current(struct collection_item *collection, parent->next = item; if (header->last == current) header->last = item; col_delete_item(current); - header->count--; + /* Deleted one added another - count stays the same! */ TRACE_FLOW_STRING("col_insert_item_into_current", "Dup overwrite exit"); return EOK; } @@ -401,7 +397,7 @@ int col_insert_item_into_current(struct collection_item *collection, parent->next = item; if (header->last == current) header->last = item; col_delete_item(current); - header->count--; + /* Deleted one added another - count stays the same! */ TRACE_FLOW_STRING("col_insert_item_into_current", "Dup overwrite exit"); return EOK; } @@ -777,7 +773,7 @@ int col_extract_item(struct collection_item *collection, struct collection_item **ret_ref) { struct collection_item *col = NULL; - int error = 0; + int error = EOK; TRACE_FLOW_STRING("col_extract_item", "Entry point"); @@ -1148,16 +1144,16 @@ static int col_create_path_data(struct path_data **name_path, TRACE_INFO_STRING("Constructing path from property:", property); /* Allocate structure */ - errno = 0; new_name_path = (struct path_data *)malloc(sizeof(struct path_data)); - if (new_name_path == NULL) return errno; - + if (new_name_path == NULL) { + TRACE_ERROR_NUMBER("Failed to allocate memory for new path struct.", ENOMEM); + return ENOMEM; + } new_name_path->name = malloc(length + property_len + 2); if (new_name_path->name == NULL) { - error = errno; - TRACE_ERROR_NUMBER("Failed to allocate memory for new path name. Errno", error); + TRACE_ERROR_NUMBER("Failed to allocate memory for new path name.", ENOMEM); free(new_name_path); - return error; + return ENOMEM; } /* Construct the new name */ @@ -1487,21 +1483,24 @@ static int col_find_item_and_do(struct collection_item *ci, return EINVAL; } + /* Collection is requered */ + if (ci == NULL) { + TRACE_ERROR_NUMBER("No collection to search!", EINVAL); + return EINVAL; + } + /* Make sure that there is anything to search */ type &= COL_TYPE_ANY; - if ((ci == NULL) || - ((property_to_find == NULL) && (type == 0)) || + if (((property_to_find == NULL) && (type == 0)) || ((*property_to_find == '\0') && (type == 0))) { - TRACE_ERROR_NUMBER("No item search criteria specified - returning error!", ENOKEY); - return ENOKEY; + TRACE_ERROR_NUMBER("No item search criteria specified - returning error!", ENOENT); + return ENOENT; } /* Prepare data for traversal */ - errno = 0; traverse_data = (struct find_name *)malloc(sizeof(struct find_name)); if (traverse_data == NULL) { - error = errno; - TRACE_ERROR_NUMBER("Failed to allocate traverse data memory - returning error!", errno); - return error; + TRACE_ERROR_NUMBER("Failed to allocate traverse data memory - returning error!", ENOMEM); + return ENOMEM; } TRACE_INFO_STRING("col_find_item_and_do", "Filling in traverse data."); @@ -2081,7 +2080,7 @@ int col_create_collection(struct collection_item **ci, const char *name, *ci = handle; TRACE_FLOW_STRING("col_create_collection", "Success Exit."); - return 0; + return EOK; } @@ -2139,6 +2138,18 @@ int col_copy_collection(struct collection_item **collection_copy, TRACE_FLOW_STRING("col_copy_collection", "Entry."); + /* Collection is requered */ + if (collection_to_copy == NULL) { + TRACE_ERROR_NUMBER("No collection to search!", EINVAL); + return EINVAL; + } + + /* Storage is required too */ + if (collection_copy == NULL) { + TRACE_ERROR_NUMBER("No memory provided to receive collection copy!", EINVAL); + return EINVAL; + } + /* Determine what name to use */ if (name_to_use != NULL) name = name_to_use; @@ -2435,6 +2446,11 @@ int col_traverse_collection(struct collection_item *ci, TRACE_FLOW_STRING("col_traverse_collection", "Entry."); + if (ci == NULL) { + TRACE_ERROR_NUMBER("No collection to traverse!", EINVAL); + return EINVAL; + } + error = col_walk_items(ci, mode_flags, col_simple_traverse_handler, NULL, item_handler, custom_data, &depth); @@ -2657,7 +2673,6 @@ static int col_grow_stack(struct collection_iterator *iterator, unsigned desired if (desired > iterator->stack_size) { grow_by = (((desired - iterator->stack_size) / STACK_DEPTH_BLOCK) + 1) * STACK_DEPTH_BLOCK; - errno = 0; temp = (struct collection_item **)realloc(iterator->stack, grow_by * sizeof(struct collection_item *)); if (temp == NULL) { TRACE_ERROR_NUMBER("Failed to allocate memory", ENOMEM); diff --git a/common/collection/collection_queue.c b/common/collection/collection_queue.c index 35c59bc49..7e0683390 100644 --- a/common/collection/collection_queue.c +++ b/common/collection/collection_queue.c @@ -19,8 +19,8 @@ along with Collection Library. If not, see . */ -#include "stdlib.h" -#include "errno.h" +#include +#include #include "collection_queue.h" #include "trace.h" diff --git a/common/collection/collection_stack.c b/common/collection/collection_stack.c index 674c6c16d..503ada337 100644 --- a/common/collection/collection_stack.c +++ b/common/collection/collection_stack.c @@ -19,8 +19,8 @@ along with Collection Library. If not, see . */ -#include "stdlib.h" -#include "errno.h" +#include +#include #include "collection_stack.h" #include "trace.h" diff --git a/common/collection/collection_tools.c b/common/collection/collection_tools.c index c6cf995c7..aa82134a3 100644 --- a/common/collection/collection_tools.c +++ b/common/collection/collection_tools.c @@ -310,11 +310,10 @@ int col_grow_buffer(struct col_serial_data *buf_data, int len) /* Grow buffer if needed */ while (buf_data->length+len >= buf_data->size) { - errno = 0; tmp = realloc(buf_data->buffer, buf_data->size + BLOCK_SIZE); if (tmp == NULL) { - TRACE_ERROR_NUMBER("Error. Failed to allocate memory. Errno: ", errno); - return errno; + TRACE_ERROR_NUMBER("Error. Failed to allocate memory.", ENOMEM); + return ENOMEM; } buf_data->buffer = tmp; buf_data->size += BLOCK_SIZE; @@ -378,11 +377,10 @@ int col_serialize(const char *property_in, } if (buf_data->buffer == NULL) { TRACE_INFO_STRING("First time use.", "Allocating buffer."); - errno = 0; buf_data->buffer = malloc(BLOCK_SIZE); if (buf_data->buffer == NULL) { - TRACE_ERROR_NUMBER("Error. Failed to allocate memory. Errno: ", errno); - return errno; + TRACE_ERROR_NUMBER("Error. Failed to allocate memory.", ENOMEM); + return ENOMEM; } buf_data->buffer[0] = '\0'; buf_data->length = 0; @@ -691,11 +689,9 @@ char **col_collection_to_list(struct collection_item *handle, int *size, int *er } /* Allocate memory for the sections */ - errno = 0; - list = (char **)calloc(count, sizeof(char *)); + list = (char **)malloc(count * sizeof(char *)); if (list == NULL) { - err = errno; - TRACE_ERROR_NUMBER("Failed to get allocate memory.", err); + TRACE_ERROR_NUMBER("Failed to get allocate memory.", ENOMEM); if (error) *error = ENOMEM; return NULL; } @@ -731,11 +727,9 @@ char **col_collection_to_list(struct collection_item *handle, int *size, int *er /* Allocate memory for the new string */ - errno = 0; list[current] = strdup(col_get_item_property(item, NULL)); if (list[current] == NULL) { - err = errno; - TRACE_ERROR_NUMBER("Failed to dup string.", err); + TRACE_ERROR_NUMBER("Failed to dup string.", ENOMEM); if (error) *error = ENOMEM; col_free_property_list(list); return NULL; @@ -743,12 +737,14 @@ char **col_collection_to_list(struct collection_item *handle, int *size, int *er current++; } + list[current] = NULL; + /* Do not forget to unbind iterator - otherwise there will be a leak */ col_unbind_iterator(iterator); if (size) *size = (int)(count - 1); if (error) *error = EOK; - TRACE_FLOW_STRING("col_collection_to_list returning", list == NULL ? "NULL" : list[0]); + TRACE_FLOW_STRING("col_collection_to_list returning", ((list == NULL) ? "NULL" : list[0])); return list; } diff --git a/common/elapi/elapi_internal.c b/common/elapi/elapi_internal.c index c5ec12474..cfc2bbeee 100644 --- a/common/elapi/elapi_internal.c +++ b/common/elapi/elapi_internal.c @@ -197,7 +197,7 @@ int elapi_internal_add_sink(struct collection_item **sink_ref, if (provider_cfg_item == NULL) { /* There is no provider - return error */ TRACE_ERROR_STRING("Required key is missing in the configuration.", "Fatal Error!"); - return ENOKEY; + return ENOENT; } @@ -275,7 +275,7 @@ int elapi_internal_create_target(struct elapi_target_context **context, if (sink_cfg_item == NULL) { /* There is no list of targets this is bad configuration - return error */ TRACE_ERROR_STRING("Required key is missing in the configuration.", "Fatal Error!"); - return ENOKEY; + return ENOENT; } /* Allocate context */ @@ -457,7 +457,7 @@ int elapi_internal_construct_target_list(struct elapi_dispatcher *handle) /* Check if we have any targets available */ if (handle->target_counter == 0) { TRACE_ERROR_STRING("No targets", ""); - return ENOKEY; + return ENOENT; } TRACE_FLOW_STRING("elapi_internal_construct_target_list", "Returning success"); diff --git a/common/elapi/elapi_log.c b/common/elapi/elapi_log.c index 122506e5a..c155fb1ac 100644 --- a/common/elapi/elapi_log.c +++ b/common/elapi/elapi_log.c @@ -202,9 +202,8 @@ int elapi_create_dispatcher_adv(struct elapi_dispatcher **dispatcher, /* Allocate memory */ handle = (struct elapi_dispatcher *) malloc(sizeof(struct elapi_dispatcher)); if (handle == NULL) { - error = errno; - TRACE_ERROR_NUMBER("Memory allocation failed. Error", error); - return error; + TRACE_ERROR_NUMBER("Memory allocation failed. Error", ENOMEM); + return ENOMEM; } /* Clean memory - we need it to be able to destroy the dispatcher at any moment */ @@ -227,10 +226,9 @@ int elapi_create_dispatcher_adv(struct elapi_dispatcher **dispatcher, /* Check error */ if (handle->appname == NULL) { - error = errno; - TRACE_ERROR_NUMBER("Memory allocation failed. Error", error); + TRACE_ERROR_NUMBER("Memory allocation failed. Error", ENOMEM); elapi_destroy_dispatcher(handle); - return error; + return ENOMEM; } /* Read the ELAPI configuration and store it in the dispatcher handle */ @@ -269,7 +267,7 @@ int elapi_create_dispatcher_adv(struct elapi_dispatcher **dispatcher, /* There is no list of targets this is bad configuration - return error */ TRACE_ERROR_STRING("No targets in the config file.", "Fatal error!"); elapi_destroy_dispatcher(handle); - return ENOKEY; + return ENOENT; } /* Get one from config but make sure we free it later */ diff --git a/common/ini/ini_config.c b/common/ini/ini_config.c index 9ef06e55a..5c89bdd41 100644 --- a/common/ini/ini_config.c +++ b/common/ini/ini_config.c @@ -632,7 +632,7 @@ int config_for_app(const char *application, /* Get specific application file */ file_name = malloc(strlen(config_dir) + strlen(application) + NAME_OVERHEAD); if (file_name == NULL) { - error = errno; + error = ENOMEM; TRACE_ERROR_NUMBER("Failed to allocate memory for file name", error); /* In case of error when we created collection - delete it */ if(error && created) { @@ -665,7 +665,6 @@ int config_for_app(const char *application, /* Add error results if any to the overarching error collection */ if ((pass_specific != NULL) && (*pass_specific != NULL)) { - TRACE_INFO_STRING("Process erros resulting from file:", file_name); error = col_add_collection_to_collection(*error_set, NULL, NULL, *pass_specific, COL_ADD_MODE_EMBED); @@ -1260,7 +1259,6 @@ char *get_string_config_value(struct collection_item *item, return NULL; } - errno = 0; str = strdup((const char *)col_get_item_data(item)); if (str == NULL) { TRACE_ERROR_NUMBER("Failed to allocate memory.", ENOMEM); @@ -1643,8 +1641,8 @@ double *get_double_config_array(struct collection_item *item, int *size, int *er /* Now parse the string */ str = (const char *)col_get_item_data(item); while (*str) { - errno = 0; TRACE_INFO_STRING("String to convert",str); + errno = 0; val = strtod(str, &endptr); if ((errno == ERANGE) || ((errno != 0) && (val == 0)) || @@ -1729,7 +1727,7 @@ char **get_section_list(struct collection_item *ini_config, int *size, int *erro /* Pass it to the function from collection API */ list = col_collection_to_list(ini_config, size, error); - TRACE_FLOW_STRING("get_section_list returning", list == NULL ? "NULL" : list[0]); + TRACE_FLOW_STRING("get_section_list returning", ((list == NULL) ? "NULL" : list[0])); return list; } @@ -1766,6 +1764,6 @@ char **get_attribute_list(struct collection_item *ini_config, const char *sectio col_destroy_collection(subcollection); - TRACE_FLOW_STRING("get_attribute_list returning", list == NULL ? "NULL" : list[0]); + TRACE_FLOW_STRING("get_attribute_list returning", ((list == NULL) ? "NULL" : list[0])); return list; } -- cgit