list: Improvements in list API

- Better documentation on header file
 - Renaming all functions that requires the list to be sorted to add
   a prefix list_sorted_
 - Make sure all functions are calling list_head() and list_tail()
   when needed

Signed-off-by: Otavio Pontes <otavio.pontes@intel.com>

src/bundle.c

@@ -161,7 +161,7 @@ int required_by(struct list **reqd_by, const char *bundle_name, struct manifest
 	if (recursion == 1) {
 		/* get rid of duplicated dependencies */
 		*reqd_by = list_sort(*reqd_by, list_strcmp);
-		*reqd_by = list_deduplicate(*reqd_by, list_strcmp, free);
+		*reqd_by = list_sorted_deduplicate(*reqd_by, list_strcmp, free);
 
 		/* if not using --verbose, we need to print the simplified
 		 * list of bundles that depend on *bundle_name */
@@ -445,7 +445,7 @@ enum swupd_code remove_bundles(struct list *bundles)
 
 		/* sanitize files to remove; if a file is needed by a bundle that
 		 * is installed, it should be kept in the system */
-		files_to_remove = list_filter_common_elements(files_to_remove, current_mom->files, filter_files_to_delete, NULL);
+		files_to_remove = list_sorted_filter_common_elements(files_to_remove, current_mom->files, filter_files_to_delete, NULL);
 
 		if (list_len(files_to_remove) > 0) {
 			info("\nDeleting bundle files...\n");

src/bundle_add.c

@@ -386,7 +386,7 @@ static struct list *generate_bundles_to_install(char **bundles)
 
 	list_free_list_and_data(aliases, free_alias_lookup);
 	bundles_list = list_sort(bundles_list, list_strcmp);
-	bundles_list = list_deduplicate(bundles_list, list_strcmp, free);
+	bundles_list = list_sorted_deduplicate(bundles_list, list_strcmp, free);
 
 	return bundles_list;
 }

src/bundle_info.c

@@ -259,8 +259,8 @@ static enum swupd_code get_bundle_dependencies(struct manifest *manifest, struct
 	*indirect_includes = list_sort(*indirect_includes, list_strcmp);
 	manifest->includes = list_sort(manifest->includes, list_strcmp);
 	manifest->optional = list_sort(manifest->optional, list_strcmp);
-	*indirect_includes = list_filter_common_elements(*indirect_includes, manifest->includes, list_strcmp, NULL);
-	*indirect_includes = list_filter_common_elements(*indirect_includes, manifest->optional, list_strcmp, NULL);
+	*indirect_includes = list_sorted_filter_common_elements(*indirect_includes, manifest->includes, list_strcmp, NULL);
+	*indirect_includes = list_sorted_filter_common_elements(*indirect_includes, manifest->optional, list_strcmp, NULL);
 
 	return SWUPD_OK;
 }

src/fullfile.c

@@ -225,7 +225,7 @@ int download_fullfiles(struct list *files, int *num_downloads)
 		/* different directories may need the same tar, in those cases it needs
 		 * to be downloaded only once, the tar for each file is downloaded from
 		 * <last_change>/files/<hash>.tar */
-		need_download = list_deduplicate(need_download, compare_fullfile, NULL);
+		need_download = list_sorted_deduplicate(need_download, compare_fullfile, NULL);
 
 		download_progress.total_download_size = fullfile_query_total_download_size(need_download);
 		if (download_progress.total_download_size > 0) {

src/lib/list.c

@@ -205,20 +205,21 @@ struct list *list_concat(struct list *list1, struct list *list2)
 	struct list *tail;
 
 	list2 = list_head(list2);
+	tail = list_tail(list1);
+	list1 = list_head(list1);
 
 	if (list1 == NULL) {
 		return list2;
 	}
 
-	list1 = list_head(list1);
-
-	if (list2) {
-		tail = list_tail(list1);
-
-		tail->next = list2;
-		list2->prev = tail;
+	if (list2 == NULL) {
+		return list1;
 	}
 
+	tail = list_tail(list1);
+	tail->next = list2;
+	list2->prev = tail;
+
 	return list1;
 }
 
@@ -314,7 +315,7 @@ int list_strcmp(const void *a, const void *b)
 	return strcmp((const char *)a, (const char *)b);
 }
 
-struct list *list_deduplicate(struct list *list, comparison_fn_t comparison_fn, list_free_data_fn_t list_free_data_fn)
+struct list *list_sorted_deduplicate(struct list *list, comparison_fn_t comparison_fn, list_free_data_fn_t list_free_data_fn)
 {
 	struct list *iter = NULL;
 	void *item1, *item2 = NULL;
@@ -359,7 +360,7 @@ struct list *list_filter_elements(struct list *list, filter_fn_t filter_fn, list
 	return list;
 }
 
-struct list *list_filter_common_elements(struct list *list1, struct list *list2, comparison_fn_t comparison_fn, list_free_data_fn_t list_free_data_fn)
+struct list *list_sorted_filter_common_elements(struct list *list1, struct list *list2, comparison_fn_t comparison_fn, list_free_data_fn_t list_free_data_fn)
 {
 	struct list *iter1, *iter2 = NULL;
 	struct list *preserver = NULL;

src/lib/list.h

@@ -4,6 +4,17 @@
 /**
  * @file
  * @brief Doubly linked list implementation.
+ *
+ * All list functions that performs operation in all nodes should always require
+ * that any element of the list is used as a parameter and return the first
+ * element of the list. Caller is not required to use list_head() in the
+ * parameter or result.
+ *
+ * List functions that operates in a single element, like list_append(),
+ * list_prepend() have documentation on what's expected from caller.
+ *
+ * A NULL pointer is always considered an empty list, so all functions should
+ * work when NULL is used as a list parameter.
  */
 
 #include <stdbool.h>
@@ -36,19 +47,30 @@ typedef void (*list_free_data_fn_t)(void *data);
 /** @brief Callback to filter a data in a list. */
 typedef bool (*filter_fn_t)(const void *a);
 
-/** @brief Callback to cone a data in a list. */
+/** @brief Callback to clone a data in a list. */
 typedef void *(*clone_fn_t)(const void *a);
 
 /**
- * Creates a new list item, store data, and inserts item in list (which can
- * be NULL). Returns created link, or NULL if failure. Created link can be
- * used as the list parameter to efficiently append new elements without
- * having to traverse the whole list to find the last one.
+ * @brief Append an element at the end of the list
+ * @param list any element of the list
+ * @param data the data the new element should keep
+ *
+ * @returns the appended element or NULL on failures.
+ *
+ * @note Created link can be used as the list parameter to efficiently append
+ * new elements without having to traverse the whole list to find the last one.
  */
 struct list *list_append_data(struct list *list, void *data);
 
 /**
  * @brief Prepend one element to the list.
+ * @param list any element of the list
+ * @param data the data the new element should keep
+ *
+ * @returns the prepended elemement or NULL on failures.
+ *
+ * @note Created link can be used as the list parameter to efficiently prepend
+ * new elements without having to traverse the whole list to find the last one.
  */
 struct list *list_prepend_data(struct list *list, void *data);
 
@@ -76,7 +98,8 @@ int list_len(struct list *list);
 struct list *list_sort(struct list *list, comparison_fn_t comparison_fn);
 
 /**
- * @brief Appends list2 at the tail of list1. Either list1 or list2 can be NULL.
+ * @brief Appends list2 at the tail of list1
+ *
  * @returns The head of the resulting concatenation
  */
 struct list *list_concat(struct list *list1, struct list *list2);
@@ -137,28 +160,17 @@ void *list_search(struct list *list, const void *item, comparison_fn_t compariso
 int list_strcmp(const void *a, const void *b);
 
 /**
- * @brief removes duplicated elements from a list
- *
- * Function requirements:
- * - the list has to be sorted
- * - the comparison_fn should behave similar to strcmp(), returning 0 if it's a match, and "< 0" or "> 0" if it's not
+ * @brief removes duplicated elements from a sorted list
  */
-struct list *list_deduplicate(struct list *list, comparison_fn_t comparison_fn, list_free_data_fn_t list_free_data_fn);
+struct list *list_sorted_deduplicate(struct list *list, comparison_fn_t comparison_fn, list_free_data_fn_t list_free_data_fn);
 
 /**
- * @brief Filters any element from list1 that happens to be in list2 as long as it meets the criteria
- *
- * Function requirements:
- * - list1 and list2 are sorted
- * - the comparison_fn should behave similar to strcmp(), returning 0 if it's a match, and "< 0" or "> 0" if it's not
+ * @brief Filters any element from sorted list list1 that happens to be in the sorted list list2 meeting the criteria.
  */
-struct list *list_filter_common_elements(struct list *list1, struct list *list2, comparison_fn_t comparison_fn, list_free_data_fn_t list_free_data_fn);
+struct list *list_sorted_filter_common_elements(struct list *list1, struct list *list2, comparison_fn_t comparison_fn, list_free_data_fn_t list_free_data_fn);
 
 /**
- * @brief removes the first occurrence of an item and returs a pointer to its data.
- *
- * Function requirements:
- * - the comparison_fn should behave similar to strcmp(), returning 0 if it's a match, and "< 0" or "> 0" if it's not
+ * @brief removes the first occurrence of an item and returns a pointer to its data.
  */
 void *list_remove(void *item_to_remove, struct list **list, comparison_fn_t comparison_fn);
 

src/verify.c

@@ -1074,7 +1074,7 @@ enum swupd_code verify_main(void)
 		 * are compared against the full list of consolidated files
 		 * from all bundles so we don't end up deleting a file that
 		 * is needed by another bundle */
-		bundles_files = list_filter_common_elements(bundles_files, all_files, find_unsafe_to_delete, NULL);
+		bundles_files = list_sorted_filter_common_elements(bundles_files, all_files, find_unsafe_to_delete, NULL);
 		official_manifest->files = bundles_files;
 
 		/* at this point we no longer need the data regarding bundles

test/unit/test_list.c

@@ -7,18 +7,18 @@
 #include "../../src/lib/list.h"
 #include "test_helper.h"
 
-void test_list_deduplicate()
+void test_list_sorted_deduplicate()
 {
 	struct list *list = NULL;
 	char *str;
 
 	// Deduplicating an empty list
-	list = list_deduplicate(NULL, list_strcmp, NULL);
+	list = list_sorted_deduplicate(NULL, list_strcmp, NULL);
 	check(list == NULL);
 
 	// Deduplicating a list of a single element
 	list = list_prepend_data(list, "A");
-	list = list_deduplicate(list, list_strcmp, NULL);
+	list = list_sorted_deduplicate(list, list_strcmp, NULL);
 
 	str = string_join(", ", list);
 	check(strcmp("A", str) == 0);
@@ -38,7 +38,7 @@ void test_list_deduplicate()
 	list = list_append_data(list, "E");
 	list = list_append_data(list, "E");
 	list = list_head(list);
-	list = list_deduplicate(list, list_strcmp, NULL);
+	list = list_sorted_deduplicate(list, list_strcmp, NULL);
 
 	str = string_join(", ", list);
 	check(strcmp("A, B, C, D, E", str) == 0);
@@ -58,7 +58,7 @@ void test_list_filter_elements()
 	char *str;
 
 	// Filter empty list
-	list = list_deduplicate(NULL, list_strcmp, NULL);
+	list = list_sorted_deduplicate(NULL, list_strcmp, NULL);
 	check(list == NULL);
 
 	// Don't remove element
@@ -117,7 +117,7 @@ void test_list_filter_elements()
 }
 
 int main() {
-	test_list_deduplicate();
+	test_list_sorted_deduplicate();
 	test_list_filter_elements();
 
 	return 0;