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.

7 files changed, 69 insertions, 62 deletions

diff --git a/common/collection/collection.c b/common/collection/collection.c
index d6fd872f..8d3b3b5d 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 35c59bc4..7e068339 100644
--- a/common/collection/collection_queue.c
+++ b/common/collection/collection_queue.c
@@ -19,8 +19,8 @@
     along with Collection Library.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-#include "stdlib.h"
-#include "errno.h"
+#include <stdlib.h>
+#include <errno.h>
 #include "collection_queue.h"
 #include "trace.h"
 
diff --git a/common/collection/collection_stack.c b/common/collection/collection_stack.c
index 674c6c16..503ada33 100644
--- a/common/collection/collection_stack.c
+++ b/common/collection/collection_stack.c
@@ -19,8 +19,8 @@
     along with Collection Library.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-#include "stdlib.h"
-#include "errno.h"
+#include <stdlib.h>
+#include <errno.h>
 #include "collection_stack.h"
 #include "trace.h"
 
diff --git a/common/collection/collection_tools.c b/common/collection/collection_tools.c
index c6cf995c..aa82134a 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 c5ec1247..cfc2bbee 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 122506e5..c155fb1a 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 9ef06e55..5c89bdd4 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;
 }