From ebe74fd253bdda11227f493efc7e919f502ea533 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 15:24:05 -0700 Subject: [PATCH 01/17] cache: free the cache table when it is empty and set to NULL We do the latter for the benefit of libpkgconf. This cleans up a significant number of memory leaks in the cache handling. --- libpkgconf/cache.c | 27 +++++++++++++++++++++++++-- libpkgconf/client.c | 2 ++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/libpkgconf/cache.c b/libpkgconf/cache.c index 90e73cb..665769f 100644 --- a/libpkgconf/cache.c +++ b/libpkgconf/cache.c @@ -16,6 +16,8 @@ #include #include +#include + /* * !doc * @@ -86,6 +88,9 @@ cache_dump(const pkgconf_client_t *client) pkgconf_pkg_t * pkgconf_cache_lookup(pkgconf_client_t *client, const char *id) { + if (client->cache_table == NULL) + return NULL; + pkgconf_pkg_t **pkg; pkg = bsearch(id, client->cache_table, @@ -150,6 +155,9 @@ pkgconf_cache_add(pkgconf_client_t *client, pkgconf_pkg_t *pkg) void pkgconf_cache_remove(pkgconf_client_t *client, pkgconf_pkg_t *pkg) { + if (client->cache_table == NULL) + return; + if (pkg == NULL) return; @@ -181,8 +189,16 @@ pkgconf_cache_remove(pkgconf_client_t *client, pkgconf_pkg_t *pkg) } client->cache_count--; - client->cache_table = pkgconf_reallocarray(client->cache_table, - client->cache_count, sizeof(void *)); + if (client->cache_count > 0) + { + client->cache_table = pkgconf_reallocarray(client->cache_table, + client->cache_count, sizeof(void *)); + } + else + { + free(client->cache_table); + client->cache_table = NULL; + } } static inline void @@ -211,6 +227,9 @@ clear_dependency_matches(pkgconf_list_t *list) void pkgconf_cache_free(pkgconf_client_t *client) { + if (client->cache_table == NULL) + return; + pkgconf_pkg_t **cache_table; size_t i, count; @@ -235,6 +254,10 @@ pkgconf_cache_free(pkgconf_client_t *client) pkgconf_pkg_t *pkg = cache_table[i]; pkgconf_pkg_free(client, pkg); } + free(cache_table); + free(client->cache_table); + client->cache_table = NULL; + client->cache_count = 0; PKGCONF_TRACE(client, "cleared package cache"); } diff --git a/libpkgconf/client.c b/libpkgconf/client.c index 05f24ec..6502ff4 100644 --- a/libpkgconf/client.c +++ b/libpkgconf/client.c @@ -98,6 +98,8 @@ pkgconf_client_init(pkgconf_client_t *client, pkgconf_error_handler_func_t error client->error_handler_data = error_handler_data; client->error_handler = error_handler; client->auditf = NULL; + client->cache_table = NULL; + client->cache_count = 0; #ifndef PKGCONF_LITE if (client->trace_handler == NULL) From a4de6235c2fea91cdb0fdb28476aa0370bb982ab Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 15:29:57 -0700 Subject: [PATCH 02/17] cache: when removing a package from the cache unset the cached flag --- libpkgconf/cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpkgconf/cache.c b/libpkgconf/cache.c index 665769f..cd07e63 100644 --- a/libpkgconf/cache.c +++ b/libpkgconf/cache.c @@ -174,7 +174,7 @@ pkgconf_cache_remove(pkgconf_client_t *client, pkgconf_pkg_t *pkg) if (slot == NULL) return; - + (*slot)->flags &= ~PKGCONF_PKG_PROPF_CACHED; *slot = NULL; qsort(client->cache_table, client->cache_count, From 4a1119aa2a9832ec9ca36d11aab6b47cc69f23b6 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 15:43:04 -0700 Subject: [PATCH 03/17] dependency: Fix reference counting of dependency_addraw We only want a reference to be added for the value inserted into the list, not the one returned. The returned one is unowned until it reaches the public dependency_add function, which returns an owned pointer instead. This makes things semantically more correct. Unfortunately, this means in a few cases we have to write some ugly code like: ```c pkgconf_dependency_t *dep = pkgcond_dependency_add("args"); pkgconf_dependency_unref(dep->owner, dep); ``` --- libpkgconf/dependency.c | 19 +++++++++++++------ libpkgconf/pkg.c | 3 ++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/libpkgconf/dependency.c b/libpkgconf/dependency.c index 08dcbb2..ff8c664 100644 --- a/libpkgconf/dependency.c +++ b/libpkgconf/dependency.c @@ -113,9 +113,16 @@ add_or_replace_dependency_node(pkgconf_client_t *client, pkgconf_dependency_t *d } PKGCONF_TRACE(client, "added dependency [%s] to list @%p; flags=%x", dependency_to_str(dep, depbuf, sizeof depbuf), list, dep->flags); - pkgconf_node_insert_tail(&dep->iter, dep, list); + pkgconf_node_insert_tail(&dep->iter, pkgconf_dependency_ref(dep->owner, dep), list); - return pkgconf_dependency_ref(dep->owner, dep); + /* This dependency is intentionally unowned. + * + * Internally we have no use for the returned type, and usually just + * discard it. However, there is a publig pkgconf_dependency_add + * function, which references this return value before returning it, + * giving ownership at that point. + */ + return dep; } static inline pkgconf_dependency_t * @@ -156,10 +163,10 @@ pkgconf_dependency_addraw(pkgconf_client_t *client, pkgconf_list_t *list, const pkgconf_dependency_t * pkgconf_dependency_add(pkgconf_client_t *client, pkgconf_list_t *list, const char *package, const char *version, pkgconf_pkg_comparator_t compare, unsigned int flags) { - if (version != NULL) - return pkgconf_dependency_addraw(client, list, package, strlen(package), version, strlen(version), compare, flags); - - return pkgconf_dependency_addraw(client, list, package, strlen(package), NULL, 0, compare, flags); + pkgconf_dependency_t *dep; + dep = pkgconf_dependency_addraw(client, list, package, strlen(package), version, + version != NULL ? strlen(version) : 0, compare, flags); + return pkgconf_dependency_ref(dep->owner, dep); } /* diff --git a/libpkgconf/pkg.c b/libpkgconf/pkg.c index 8be4ca7..af68943 100644 --- a/libpkgconf/pkg.c +++ b/libpkgconf/pkg.c @@ -451,7 +451,8 @@ pkgconf_pkg_new_from_file(pkgconf_client_t *client, const char *filename, FILE * return NULL; } - pkgconf_dependency_add(client, &pkg->provides, pkg->id, pkg->version, PKGCONF_CMP_EQUAL, 0); + pkgconf_dependency_t *dep = pkgconf_dependency_add(client, &pkg->provides, pkg->id, pkg->version, PKGCONF_CMP_EQUAL, 0); + pkgconf_dependency_unref(dep->owner, dep); return pkgconf_pkg_ref(client, pkg); } From e275594ba697901921da86ded8819a4f4fb10c82 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 15:56:54 -0700 Subject: [PATCH 04/17] queue: ensure cleanup happens when applying --- libpkgconf/queue.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libpkgconf/queue.c b/libpkgconf/queue.c index 61584d4..1c30f40 100644 --- a/libpkgconf/queue.c +++ b/libpkgconf/queue.c @@ -261,6 +261,7 @@ pkgconf_queue_verify(pkgconf_client_t *client, pkgconf_pkg_t *world, pkgconf_lis bool pkgconf_queue_apply(pkgconf_client_t *client, pkgconf_list_t *list, pkgconf_queue_apply_func_t func, int maxdepth, void *data) { + bool ret = false; pkgconf_pkg_t world = { .id = "virtual:world", .realname = "virtual world package", @@ -272,18 +273,17 @@ pkgconf_queue_apply(pkgconf_client_t *client, pkgconf_list_t *list, pkgconf_queu maxdepth = -1; if (pkgconf_queue_verify(client, &world, list, maxdepth) != PKGCONF_PKG_ERRF_OK) - return false; + goto cleanup; /* the world dependency set is flattened after it is returned from pkgconf_queue_verify */ if (!func(client, &world, data, maxdepth)) - { - pkgconf_pkg_free(client, &world); - return false; - } + goto cleanup; + ret = true; + +cleanup: pkgconf_pkg_free(client, &world); - - return true; + return ret; } /* From 6609001114daa3bf3780e944af669b627fb5a174 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 16:08:00 -0700 Subject: [PATCH 05/17] queue: unref dependency in all cases --- libpkgconf/queue.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libpkgconf/queue.c b/libpkgconf/queue.c index 1c30f40..7e1d011 100644 --- a/libpkgconf/queue.c +++ b/libpkgconf/queue.c @@ -160,7 +160,7 @@ flatten_dependency_set(pkgconf_client_t *client, pkgconf_list_t *list) continue; if (pkg->serial == client->serial) - continue; + goto next; if (dep->match == NULL) { @@ -190,8 +190,8 @@ flatten_dependency_set(pkgconf_client_t *client, pkgconf_list_t *list) deps[dep_count - 1] = dep; PKGCONF_TRACE(client, "added %s to dep table", dep->package); - -next:; +next: + pkgconf_pkg_unref(client, pkg); } qsort(deps, dep_count, sizeof (void *), dep_sort_cmp); From e4d1c8ffa5caa2a2e0064fcc7e5d9ce89490335e Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 16:22:14 -0700 Subject: [PATCH 06/17] queue: when flattening do nothing if the flattened deps are empty --- libpkgconf/queue.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libpkgconf/queue.c b/libpkgconf/queue.c index 7e1d011..13f93e4 100644 --- a/libpkgconf/queue.c +++ b/libpkgconf/queue.c @@ -194,6 +194,9 @@ next: pkgconf_pkg_unref(client, pkg); } + if (deps == NULL) + return; + qsort(deps, dep_count, sizeof (void *), dep_sort_cmp); /* zero the list and start readding */ From 171738024e2cf30a8875b56909596a22ea85f297 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 16:27:42 -0700 Subject: [PATCH 07/17] cache: clear the cache with pkgconf_cache_remove Which results in more code re-use. --- libpkgconf/cache.c | 40 ++++------------------------------------ 1 file changed, 4 insertions(+), 36 deletions(-) diff --git a/libpkgconf/cache.c b/libpkgconf/cache.c index cd07e63..fee0f28 100644 --- a/libpkgconf/cache.c +++ b/libpkgconf/cache.c @@ -174,7 +174,9 @@ pkgconf_cache_remove(pkgconf_client_t *client, pkgconf_pkg_t *pkg) if (slot == NULL) return; + (*slot)->flags &= ~PKGCONF_PKG_PROPF_CACHED; + pkgconf_pkg_unref(client, *slot); *slot = NULL; qsort(client->cache_table, client->cache_count, @@ -201,18 +203,6 @@ pkgconf_cache_remove(pkgconf_client_t *client, pkgconf_pkg_t *pkg) } } -static inline void -clear_dependency_matches(pkgconf_list_t *list) -{ - pkgconf_node_t *iter; - - PKGCONF_FOREACH_LIST_ENTRY(list->head, iter) - { - pkgconf_dependency_t *dep = iter->data; - dep->match = NULL; - } -} - /* * !doc * @@ -230,31 +220,9 @@ pkgconf_cache_free(pkgconf_client_t *client) if (client->cache_table == NULL) return; - pkgconf_pkg_t **cache_table; - size_t i, count; + while (client->cache_count > 0) + pkgconf_cache_remove(client, client->cache_table[0]); - cache_table = pkgconf_reallocarray(NULL, client->cache_count, sizeof (void *)); - memcpy(cache_table, client->cache_table, - client->cache_count * sizeof (void *)); - - /* first we clear cached match pointers */ - for (i = 0, count = client->cache_count; i < count; i++) - { - pkgconf_pkg_t *pkg = cache_table[i]; - - clear_dependency_matches(&pkg->required); - clear_dependency_matches(&pkg->requires_private); - clear_dependency_matches(&pkg->provides); - clear_dependency_matches(&pkg->conflicts); - } - - /* now forcibly free everything */ - for (i = 0, count = client->cache_count; i < count; i++) - { - pkgconf_pkg_t *pkg = cache_table[i]; - pkgconf_pkg_free(client, pkg); - } - free(cache_table); free(client->cache_table); client->cache_table = NULL; client->cache_count = 0; From 38103134a5c893a0686d49a0953598268c2aff2b Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 16:30:59 -0700 Subject: [PATCH 08/17] main: goto cleanup in validate case too This fixes leaks in two tests --- cli/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/main.c b/cli/main.c index 5faf0dc..b37a37a 100644 --- a/cli/main.c +++ b/cli/main.c @@ -1337,7 +1337,7 @@ main(int argc, char *argv[]) } if ((want_flags & PKG_VALIDATE) == PKG_VALIDATE) - return 0; + goto out; if ((want_flags & PKG_UNINSTALLED) == PKG_UNINSTALLED) { From a391f9b650b3d7bfb55227fec37edfc53e85b69b Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 16:32:35 -0700 Subject: [PATCH 09/17] pkg: use goto cleanup idiom --- libpkgconf/pkg.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libpkgconf/pkg.c b/libpkgconf/pkg.c index af68943..3eb67f2 100644 --- a/libpkgconf/pkg.c +++ b/libpkgconf/pkg.c @@ -1470,21 +1470,18 @@ pkgconf_pkg_walk_list(pkgconf_client_t *client, if (pkgdep->serial == client->serial) { pkgdep->hits++; - pkgconf_pkg_unref(client, pkgdep); - continue; + goto next; } if (skip_flags && (depnode->flags & skip_flags) == skip_flags) - { - pkgconf_pkg_unref(client, pkgdep); - continue; - } + goto next; pkgconf_audit_log_dependency(client, pkgdep, depnode); pkgdep->hits++; pkgdep->serial = client->serial; eflags |= pkgconf_pkg_traverse_main(client, pkgdep, func, data, depth - 1, skip_flags); +next: pkgconf_pkg_unref(client, pkgdep); } From 4493a322f67f7e2df68337912c06c21939680ffa Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 16:37:04 -0700 Subject: [PATCH 10/17] main: do cleanup when checking required version --- cli/main.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/cli/main.c b/cli/main.c index b37a37a..b380c43 100644 --- a/cli/main.c +++ b/cli/main.c @@ -1183,17 +1183,21 @@ main(int argc, char *argv[]) pkgconf_error(&pkg_client, "Package '%s' was not found\n", pkgiter->package); ret = EXIT_FAILURE; - goto out; + goto cleanup; } if (pkgconf_compare_version(pkg->version, required_module_version) >= 0) { ret = EXIT_SUCCESS; - goto out; + goto cleanup; } } ret = EXIT_FAILURE; +cleanup: + if (pkg) + pkgconf_pkg_unref(&pkg_client, pkg); + pkgconf_dependency_free(&deplist); goto out; } else if (required_exact_module_version != NULL) @@ -1219,17 +1223,21 @@ main(int argc, char *argv[]) pkgconf_error(&pkg_client, "Package '%s' was not found\n", pkgiter->package); ret = EXIT_FAILURE; - goto out; + goto cleanup2; } if (pkgconf_compare_version(pkg->version, required_exact_module_version) == 0) { ret = EXIT_SUCCESS; - goto out; + goto cleanup2; } } ret = EXIT_FAILURE; +cleanup2: + if (pkg) + pkgconf_pkg_unref(&pkg_client, pkg); + pkgconf_dependency_free(&deplist); goto out; } else if (required_max_module_version != NULL) @@ -1255,17 +1263,21 @@ main(int argc, char *argv[]) pkgconf_error(&pkg_client, "Package '%s' was not found\n", pkgiter->package); ret = EXIT_FAILURE; - goto out; + goto cleanup3; } if (pkgconf_compare_version(pkg->version, required_max_module_version) <= 0) { ret = EXIT_SUCCESS; - goto out; + goto cleanup3; } } ret = EXIT_FAILURE; +cleanup3: + if (pkg) + pkgconf_pkg_unref(&pkg_client, pkg); + pkgconf_dependency_free(&deplist); goto out; } From a46ce3672fab56d6e1ee1c1e493fda62dbf8e13b Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 16:40:04 -0700 Subject: [PATCH 11/17] queue: when collecting dependents don't iterate private twice Currently, the private field is iterated collecting private deps and normal deps. It should only be iterated when collecting private deps. --- libpkgconf/queue.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/libpkgconf/queue.c b/libpkgconf/queue.c index 13f93e4..c353e7e 100644 --- a/libpkgconf/queue.c +++ b/libpkgconf/queue.c @@ -116,22 +116,27 @@ pkgconf_queue_collect_dependents(pkgconf_client_t *client, pkgconf_pkg_t *pkg, v if (pkg == world) return; - PKGCONF_FOREACH_LIST_ENTRY(pkg->required.head, node) + if (!(pkg->flags & PKGCONF_PKG_PKGF_SEARCH_PRIVATE)) { - pkgconf_dependency_t *flattened_dep; + PKGCONF_FOREACH_LIST_ENTRY(pkg->required.head, node) + { + pkgconf_dependency_t *flattened_dep; - flattened_dep = pkgconf_dependency_copy(client, node->data); + flattened_dep = pkgconf_dependency_copy(client, node->data); - pkgconf_node_insert(&flattened_dep->iter, flattened_dep, &world->required); + pkgconf_node_insert(&flattened_dep->iter, flattened_dep, &world->required); + } } - - PKGCONF_FOREACH_LIST_ENTRY(pkg->requires_private.head, node) + else { - pkgconf_dependency_t *flattened_dep; + PKGCONF_FOREACH_LIST_ENTRY(pkg->requires_private.head, node) + { + pkgconf_dependency_t *flattened_dep; - flattened_dep = pkgconf_dependency_copy(client, node->data); + flattened_dep = pkgconf_dependency_copy(client, node->data); - pkgconf_node_insert(&flattened_dep->iter, flattened_dep, &world->requires_private); + pkgconf_node_insert(&flattened_dep->iter, flattened_dep, &world->requires_private); + } } } From 4934205737adcae79b9c202b1502e61f0f223c74 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 4 Aug 2022 11:50:50 -0700 Subject: [PATCH 12/17] pkg: add name of pkg being refed/unrefed to debug outpu --- libpkgconf/pkg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libpkgconf/pkg.c b/libpkgconf/pkg.c index 3eb67f2..2555e59 100644 --- a/libpkgconf/pkg.c +++ b/libpkgconf/pkg.c @@ -537,7 +537,7 @@ pkgconf_pkg_ref(pkgconf_client_t *client, pkgconf_pkg_t *pkg) PKGCONF_TRACE(client, "WTF: client %p refers to package %p owned by other client %p", client, pkg, pkg->owner); pkg->refcount++; - PKGCONF_TRACE(client, "refcount@%p: %d", pkg, pkg->refcount); + PKGCONF_TRACE(client, "%s refcount@%p: %d", pkg->id, pkg, pkg->refcount); return pkg; } @@ -560,7 +560,7 @@ pkgconf_pkg_unref(pkgconf_client_t *client, pkgconf_pkg_t *pkg) PKGCONF_TRACE(client, "WTF: client %p unrefs package %p owned by other client %p", client, pkg, pkg->owner); pkg->refcount--; - PKGCONF_TRACE(pkg->owner, "refcount@%p: %d", pkg, pkg->refcount); + PKGCONF_TRACE(pkg->owner, "%s refcount@%p: %d", pkg->id, pkg, pkg->refcount); if (pkg->refcount <= 0) pkgconf_pkg_free(pkg->owner, pkg); From e71a5a33709e5c4977ed69842ef30882cc5c76b3 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 4 Aug 2022 11:52:26 -0700 Subject: [PATCH 13/17] dependency: add debug information for dependency refcounting --- libpkgconf/dependency.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libpkgconf/dependency.c b/libpkgconf/dependency.c index ff8c664..49856b3 100644 --- a/libpkgconf/dependency.c +++ b/libpkgconf/dependency.c @@ -229,6 +229,7 @@ pkgconf_dependency_ref(pkgconf_client_t *client, pkgconf_dependency_t *dep) return NULL; dep->refcount++; + PKGCONF_TRACE(client, "%s refcount@%p: %d", dep->package, dep, dep->refcount); return dep; } @@ -249,7 +250,10 @@ pkgconf_dependency_unref(pkgconf_client_t *client, pkgconf_dependency_t *dep) if (client != dep->owner) return; - if (--dep->refcount <= 0) + --dep->refcount; + PKGCONF_TRACE(client, "%s refcount@%p: %d", dep->package, dep, dep->refcount); + + if (dep->refcount <= 0) pkgconf_dependency_free_one(dep); } From 34b110200aeaab1d8dd245d9d1cf9ebabe6e18c7 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 4 Aug 2022 11:53:02 -0700 Subject: [PATCH 14/17] dependency: zero list after freeing --- libpkgconf/dependency.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libpkgconf/dependency.c b/libpkgconf/dependency.c index 49856b3..149d95b 100644 --- a/libpkgconf/dependency.c +++ b/libpkgconf/dependency.c @@ -279,6 +279,8 @@ pkgconf_dependency_free(pkgconf_list_t *list) pkgconf_node_delete(&dep->iter, list); pkgconf_dependency_unref(dep->owner, dep); } + + pkgconf_list_zero(list); } /* From 301d8fa0c6cd8af2dc89a553191f1ceb1ed35535 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 4 Aug 2022 12:22:49 -0700 Subject: [PATCH 15/17] queue: free unused dependencies when flattening --- libpkgconf/queue.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libpkgconf/queue.c b/libpkgconf/queue.c index c353e7e..34c33b6 100644 --- a/libpkgconf/queue.c +++ b/libpkgconf/queue.c @@ -152,11 +152,11 @@ dep_sort_cmp(const void *a, const void *b) static inline void flatten_dependency_set(pkgconf_client_t *client, pkgconf_list_t *list) { - pkgconf_node_t *node; + pkgconf_node_t *node, *next; pkgconf_dependency_t **deps = NULL; size_t dep_count = 0, i; - PKGCONF_FOREACH_LIST_ENTRY(list->head, node) + PKGCONF_FOREACH_LIST_ENTRY_SAFE(list->head, next, node) { pkgconf_dependency_t *dep = node->data; pkgconf_pkg_t *pkg = pkgconf_pkg_verify_dependency(client, dep, NULL); @@ -165,7 +165,11 @@ flatten_dependency_set(pkgconf_client_t *client, pkgconf_list_t *list) continue; if (pkg->serial == client->serial) + { + pkgconf_node_delete(node, list); + pkgconf_dependency_unref(client, dep); goto next; + } if (dep->match == NULL) { From 1cfa2d1e20d9a55906580c5e3f82c3c49d2516e5 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 4 Aug 2022 15:16:44 -0700 Subject: [PATCH 16/17] pkg: prevent circular ownership Otherwise in a case where A references B, and B references A, A and B will have mutual ownership of each other and prevent each other from being free'd. --- libpkgconf/pkg.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/libpkgconf/pkg.c b/libpkgconf/pkg.c index 2555e59..5f2f603 100644 --- a/libpkgconf/pkg.c +++ b/libpkgconf/pkg.c @@ -1445,9 +1445,9 @@ pkgconf_pkg_walk_list(pkgconf_client_t *client, unsigned int skip_flags) { unsigned int eflags = PKGCONF_PKG_ERRF_OK; - pkgconf_node_t *node; + pkgconf_node_t *node, *next; - PKGCONF_FOREACH_LIST_ENTRY(deplist->head, node) + PKGCONF_FOREACH_LIST_ENTRY_SAFE(deplist->head, next, node) { unsigned int eflags_local = PKGCONF_PKG_ERRF_OK; pkgconf_dependency_t *depnode = node->data; @@ -1470,6 +1470,14 @@ pkgconf_pkg_walk_list(pkgconf_client_t *client, if (pkgdep->serial == client->serial) { pkgdep->hits++; + /* In this case we have a circular reference. + * We break that by deleteing the circular node from the + * the list, so that we dont create a situation where + * memory is leaked due to circular ownership. + * i.e: A owns B owns A + */ + pkgconf_node_delete(node, deplist); + pkgconf_dependency_unref(client, depnode); goto next; } From 179a0560a6b57119e5e9b72ce295c283d12a7b5d Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 27 Jul 2022 14:22:54 -0700 Subject: [PATCH 17/17] ci: run meson test with the address sanitizer enabled Set the ASAN_OPTION so that the exitcode is not 1, this avoids asan returning the exitcode that the tests already expect from a normal pkgconf error --- .github/workflows/test.yml | 24 ++++++++++++++++++++++++ .woodpecker.yml | 15 +++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8ebe9f1..46277af 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -60,6 +60,30 @@ jobs: run: | meson test -v -C _build + debian-meson-asan: + runs-on: ubuntu-latest + container: + image: debian:testing + steps: + - name: Checkout + uses: actions/checkout@v2 + + - name: Update system and add dependencies + run: | + apt-get update + apt-get install -y kyua atf-sh build-essential meson + + - name: Build + run: | + meson _build -Db_sanitize=address + meson compile -C _build + + - name: Run tests + run: | + meson test -v -C _build + env: + ASAN_OPTIONS: "exitcode=7" + debian-autotools: runs-on: ubuntu-latest container: diff --git a/.woodpecker.yml b/.woodpecker.yml index 2304cf7..65cd986 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -12,6 +12,21 @@ pipeline: IMAGE: debian BUILD: meson + debian-meson-asan: + image: debian:testing + environment: + - ASAN_OPTIONS="exitcode=7" + commands: + - apt-get update + - apt-get install -y kyua atf-sh build-essential meson + - meson _build -Db_sanitize=address + - meson compile -C _build + - meson test -v -C _build + when: + matrix: + IMAGE: debian + BUILD: meson + debian-autotools: image: debian:testing commands: