From 4a1119aa2a9832ec9ca36d11aab6b47cc69f23b6 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Wed, 3 Aug 2022 15:43:04 -0700 Subject: [PATCH] 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); }