CLEANUP: resolvers: simplify resolv_link_resolution() regarding requesters
This function allocates requesters by hand for each and every type. This
is complex and error-prone, and it doesn't even initialize the list part,
leaving dangling pointers that complicate debugging.
This patch introduces a new function resolv_get_requester() that either
returns the current pointer if valid or tries to allocate a new one and
links it to its destination. Then it makes use of it in the function
above to clean it up quite a bit. This allows to remove complicated but
unneeded tests.
(cherry picked from commit 239675e4a955b219e915fa11a1a03c7aacc13ccd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/src/resolvers.c b/src/resolvers.c
index 75d1419..b0bd5af 100644
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -1857,6 +1857,35 @@
pool_free(resolv_resolution_pool, resolution);
}
+/* If *<req> is not NULL, returns it, otherwise tries to allocate a requester
+ * and makes it owned by this obj_type, with the proposed callback and error
+ * callback. On success, *req is assigned the allocated requester. Returns
+ * NULL on allocation failure.
+ */
+static struct resolv_requester *
+resolv_get_requester(struct resolv_requester **req, enum obj_type *owner,
+ int (*cb)(struct resolv_requester *, struct dns_counters *),
+ int (*err_cb)(struct resolv_requester *, int))
+{
+ struct resolv_requester *tmp;
+
+ if (*req)
+ return *req;
+
+ tmp = pool_alloc(resolv_requester_pool);
+ if (!tmp)
+ goto end;
+
+ LIST_INIT(&tmp->list);
+ tmp->owner = owner;
+ tmp->resolution = NULL;
+ tmp->requester_cb = cb;
+ tmp->requester_error_cb = err_cb;
+ *req = tmp;
+ end:
+ return tmp;
+}
+
/* Links a requester (a server or a resolv_srvrq) with a resolution. It returns 0
* on success, -1 otherwise.
*/
@@ -1874,6 +1903,21 @@
switch (requester_type) {
case OBJ_TYPE_SERVER:
srv = (struct server *)requester;
+
+ if (!requester_locked)
+ HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
+
+ req = resolv_get_requester(&srv->resolv_requester,
+ &srv->obj_type,
+ snr_resolution_cb,
+ snr_resolution_error_cb);
+
+ if (!requester_locked)
+ HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+
+ if (!req)
+ goto err;
+
hostname_dn = &srv->hostname_dn;
hostname_dn_len = srv->hostname_dn_len;
resolvers = srv->resolvers;
@@ -1884,6 +1928,14 @@
case OBJ_TYPE_SRVRQ:
srvrq = (struct resolv_srvrq *)requester;
+
+ req = resolv_get_requester(&srvrq->requester,
+ &srvrq->obj_type,
+ snr_resolution_cb,
+ srvrq_resolution_error_cb);
+ if (!req)
+ goto err;
+
hostname_dn = &srvrq->hostname_dn;
hostname_dn_len = srvrq->hostname_dn_len;
resolvers = srvrq->resolvers;
@@ -1892,6 +1944,14 @@
case OBJ_TYPE_STREAM:
stream = (struct stream *)requester;
+
+ req = resolv_get_requester(&stream->resolv_ctx.requester,
+ &stream->obj_type,
+ act_resolution_cb,
+ act_resolution_error_cb);
+ if (!req)
+ goto err;
+
hostname_dn = &stream->resolv_ctx.hostname_dn;
hostname_dn_len = stream->resolv_ctx.hostname_dn_len;
resolvers = stream->resolv_ctx.parent->arg.resolv.resolvers;
@@ -1907,56 +1967,7 @@
if ((res = resolv_pick_resolution(resolvers, hostname_dn, hostname_dn_len, query_type)) == NULL)
goto err;
- if (srv) {
- if (!requester_locked)
- HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
- if (srv->resolv_requester == NULL) {
- if ((req = pool_alloc(resolv_requester_pool)) == NULL) {
- if (!requester_locked)
- HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
- goto err;
- }
- req->owner = &srv->obj_type;
- srv->resolv_requester = req;
- }
- else
- req = srv->resolv_requester;
- if (!requester_locked)
- HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
-
- req->requester_cb = snr_resolution_cb;
- req->requester_error_cb = snr_resolution_error_cb;
- }
- else if (srvrq) {
- if (srvrq->requester == NULL) {
- if ((req = pool_alloc(resolv_requester_pool)) == NULL)
- goto err;
- req->owner = &srvrq->obj_type;
- srvrq->requester = req;
- }
- else
- req = srvrq->requester;
-
- req->requester_cb = snr_resolution_cb;
- req->requester_error_cb = srvrq_resolution_error_cb;
- }
- else if (stream) {
- if (stream->resolv_ctx.requester == NULL) {
- if ((req = pool_alloc(resolv_requester_pool)) == NULL)
- goto err;
- req->owner = &stream->obj_type;
- stream->resolv_ctx.requester = req;
- }
- else
- req = stream->resolv_ctx.requester;
-
- req->requester_cb = act_resolution_cb;
- req->requester_error_cb = act_resolution_error_cb;
- }
- else
- goto err;
-
- req->resolution = res;
+ req->resolution = res;
LIST_APPEND(&res->requesters, &req->list);
return 0;