From 96061a06d75049314d2116a5fa41b73739996cf7 Mon Sep 17 00:00:00 2001 From: Otavio Pontes Date: Fri, 25 Oct 2019 09:16:53 -0700 Subject: [PATCH] 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 --- src/bundle.c | 4 ++-- src/bundle_add.c | 2 +- src/bundle_info.c | 4 ++-- src/fullfile.c | 2 +- src/lib/list.c | 19 ++++++++------- src/lib/list.h | 56 ++++++++++++++++++++++++++----------------- src/verify.c | 2 +- test/unit/test_list.c | 12 +++++----- 8 files changed, 57 insertions(+), 44 deletions(-) diff --git a/src/bundle.c b/src/bundle.c index 3044fa34..5ca31a59 100644 --- a/src/bundle.c +++ b/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"); diff --git a/src/bundle_add.c b/src/bundle_add.c index cceaf6b1..9272547b 100644 --- a/src/bundle_add.c +++ b/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; } diff --git a/src/bundle_info.c b/src/bundle_info.c index 25bd4096..0c82dec5 100644 --- a/src/bundle_info.c +++ b/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; } diff --git a/src/fullfile.c b/src/fullfile.c index 4d94d8b7..5bd01ced 100644 --- a/src/fullfile.c +++ b/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 * /files/.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) { diff --git a/src/lib/list.c b/src/lib/list.c index 19722838..a8a1331d 100644 --- a/src/lib/list.c +++ b/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; diff --git a/src/lib/list.h b/src/lib/list.h index eafa6b7d..45375457 100644 --- a/src/lib/list.h +++ b/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 @@ -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); diff --git a/src/verify.c b/src/verify.c index dd0f5565..e8b2f3ed 100644 --- a/src/verify.c +++ b/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 diff --git a/test/unit/test_list.c b/test/unit/test_list.c index 1dc50e2d..b1d2b713 100644 --- a/test/unit/test_list.c +++ b/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;