unwind(8): refactor & simplify refcounting

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

unwind(8): refactor & simplify refcounting

Florian Obser-2
Did I get this right? I'd appreciate it if someone could give this a
once over.

    Since resolve() switched to a callback mechanism all uw_resolver objects
    pass through resolve() and either asr_resolve_done() or
    ub_resolve_done().
    With that we can pull resolver_ref() and resolver_unref() into those
    functions to make the reference counting easier.
    Only check_resolver is special since it needs to refcount the to be
    checked resolver. But the resolver doing the actual work is
    automatically refcounted by resolve() and *_resolve_done().
    One last piece of the puzzle is to track the uw_resolver object in
    cb_data so that the *_resolve_done() functions have access to it.
    This also allowes us to remove the ad-hoc passing of the resolver in
    query_imsg. Since the callback functions all need access to the
    resolver that did the work we pass it in as first argument.

diff --git resolver.c resolver.c
index d1dce2dec71..2b7d81d29fc 100644
--- resolver.c
+++ resolver.c
@@ -92,14 +92,11 @@ struct uw_resolver {
  int64_t histogram[nitems(histogram_limits)];
 };
 
-struct check_resolver_data {
- struct uw_resolver *res;
- struct uw_resolver *check_res;
-};
-
 struct resolver_cb_data {
- void (*cb)(void *, int, void *, int, int, char *);
- void *data;
+ void (*cb)(struct uw_resolver *, void *, int, void *,
+        int, int, char *);
+ void *data;
+ struct uw_resolver *res;
 };
 
 __dead void resolver_shutdown(void);
@@ -108,9 +105,10 @@ void resolver_dispatch_frontend(int, short, void *);
 void resolver_dispatch_captiveportal(int, short, void *);
 void resolver_dispatch_main(int, short, void *);
 int resolve(struct uw_resolver *, const char*, int, int,
-     void*, void (*cb)(void *, int, void *, int, int,
-     char *));
-void resolve_done(void *, int, void *, int, int, char *);
+     void*, void (*cb)(struct uw_resolver *, void *,
+     int, void *, int, int, char *));
+void resolve_done(struct uw_resolver *, void *, int, void *,
+     int, int, char *);
 void ub_resolve_done(void *, int, void *, int, int, char *,
      int);
 void asr_resolve_done(struct asr_result *, void *);
@@ -129,8 +127,8 @@ void set_forwarders_oppdot(struct uw_resolver *,
 void resolver_check_timo(int, short, void *);
 void resolver_free_timo(int, short, void *);
 void check_resolver(struct uw_resolver *);
-void check_resolver_done(void *, int, void *, int, int,
-     char *);
+void check_resolver_done(struct uw_resolver *, void *, int,
+     void *, int, int, char *);
 void schedule_recheck_all_resolvers(void);
 int check_forwarders_changed(struct uw_forwarder_head *,
      struct uw_forwarder_head *);
@@ -154,8 +152,8 @@ int check_captive_portal_changed(struct uw_conf *,
      struct uw_conf *);
 void trust_anchor_resolve(void);
 void trust_anchor_timo(int, short, void *);
-void trust_anchor_resolve_done(void *, int, void *, int,
-     int, char *);
+void trust_anchor_resolve_done(struct uw_resolver *, void *,
+     int, void *, int, int, char *);
 void add_autoconf_forwarders(struct imsg_rdns_proposal *);
 void rem_autoconf_forwarders(struct imsg_rdns_proposal *);
 struct uw_forwarder *find_forwarder(struct uw_forwarder_head *,
@@ -480,14 +478,10 @@ resolver_dispatch_frontend(int fd, short event, void *bula)
  log_debug("%s: choosing %s", __func__,
     uw_resolver_type_str[res->type]);
 
- query_imsg->resolver = res;
- resolver_ref(res);
-
  clock_gettime(CLOCK_MONOTONIC, &query_imsg->tp);
 
- if ((resolve(res, query_imsg->qname, query_imsg->t,
-    query_imsg->c, query_imsg, resolve_done)) != 0)
- resolver_unref(res);
+ resolve(res, query_imsg->qname, query_imsg->t,
+    query_imsg->c, query_imsg, resolve_done);
  break;
  case IMSG_FORWARDER:
  /* make sure this is a string */
@@ -773,16 +767,20 @@ resolver_dispatch_main(int fd, short event, void *bula)
 
 int
 resolve(struct uw_resolver *res, const char* name, int rrtype, int rrclass,
-    void *mydata, void (*cb)(void *, int, void *, int, int, char *))
+    void *mydata, void (*cb)(struct uw_resolver *, void *, int, void *, int,
+    int, char *))
 {
  struct resolver_cb_data *cb_data = NULL;
  struct asr_query *aq = NULL;
  int err;
 
+ resolver_ref(res);
+
  if ((cb_data = malloc(sizeof(*cb_data))) == NULL)
  goto err;
  cb_data->cb = cb;
  cb_data->data = mydata;
+ cb_data->res = res;
 
  switch(res->type) {
  case UW_RES_ASR:
@@ -816,15 +814,15 @@ resolve(struct uw_resolver *res, const char* name, int rrtype, int rrclass,
  err:
  free(cb_data);
  free(aq);
+ resolver_unref(res);
  return 1;
 }
 
 void
-resolve_done(void *arg, int rcode, void *answer_packet, int answer_len,
-    int sec, char *why_bogus)
+resolve_done(struct uw_resolver *res, void *arg, int rcode,
+    void *answer_packet, int answer_len, int sec, char *why_bogus)
 {
  struct query_imsg *query_imsg;
- struct uw_resolver *res;
  struct timespec tp, elapsed;
  int64_t ms;
  size_t i;
@@ -833,7 +831,6 @@ resolve_done(void *arg, int rcode, void *answer_packet, int answer_len,
  clock_gettime(CLOCK_MONOTONIC, &tp);
 
  query_imsg = (struct query_imsg *)arg;
- res = (struct uw_resolver *)query_imsg->resolver;
 
  timespecsub(&tp, &query_imsg->tp, &elapsed);
 
@@ -886,7 +883,6 @@ resolve_done(void *arg, int rcode, void *answer_packet, int answer_len,
     answer_packet, answer_len);
 
  free(query_imsg);
- resolver_unref(res);
  return;
 
 servfail:
@@ -894,7 +890,6 @@ servfail:
  resolver_imsg_compose_frontend(IMSG_ANSWER_HEADER, 0, query_imsg,
     sizeof(*query_imsg));
  free(query_imsg);
- resolver_unref(res);
 }
 
 void
@@ -1259,95 +1254,83 @@ resolver_free_timo(int fd, short events, void *arg)
 }
 
 void
-check_resolver(struct uw_resolver *res)
+check_resolver(struct uw_resolver *resolver_to_check)
 {
- struct uw_resolver *check_res;
- struct check_resolver_data *data;
+ struct uw_resolver *res;
 
  log_debug("%s: create_resolver", __func__);
- if ((check_res = create_resolver(res->type, 0)) == NULL)
- fatal("%s", __func__);
- if ((data = malloc(sizeof(*data))) == NULL)
+ if ((res = create_resolver(resolver_to_check->type, 0)) == NULL)
  fatal("%s", __func__);
 
- resolver_ref(check_res);
- resolver_ref(res);
- data->check_res = check_res;
- data->res = res;
+ resolver_ref(resolver_to_check);
 
- if (resolve(check_res, ".", LDNS_RR_TYPE_NS, LDNS_RR_CLASS_IN,
-    data, check_resolver_done) != 0) {
- res->state = UNKNOWN;
- resolver_unref(check_res);
- resolver_unref(res);
- res->check_tv.tv_sec = RESOLVER_CHECK_SEC;
- evtimer_add(&res->check_ev, &res->check_tv);
+ if (resolve(res, ".", LDNS_RR_TYPE_NS, LDNS_RR_CLASS_IN,
+    resolver_to_check, check_resolver_done) != 0) {
+ resolver_to_check->state = UNKNOWN;
+ resolver_unref(resolver_to_check);
+ resolver_to_check->check_tv.tv_sec = RESOLVER_CHECK_SEC;
+ evtimer_add(&resolver_to_check->check_ev,
+    &resolver_to_check->check_tv);
 
  log_debug("%s: evtimer_add: %lld - %s: %s", __func__,
-    data->res->check_tv.tv_sec,
-    uw_resolver_type_str[data->res->type],
-    uw_resolver_state_str[data->res->state]);
+    resolver_to_check->check_tv.tv_sec,
+    uw_resolver_type_str[resolver_to_check->type],
+    uw_resolver_state_str[resolver_to_check->state]);
  }
 
- if (!(res->type == UW_RES_DHCP || res->type == UW_RES_FORWARDER))
+ if (!(resolver_to_check->type == UW_RES_DHCP ||
+    resolver_to_check->type == UW_RES_FORWARDER))
  return;
 
  log_debug("%s: create_resolver for oppdot", __func__);
- if ((check_res = create_resolver(res->type, 1)) == NULL)
- fatal("%s", __func__);
- if ((data = malloc(sizeof(*data))) == NULL)
+ if ((res = create_resolver(resolver_to_check->type, 1)) == NULL)
  fatal("%s", __func__);
 
- resolver_ref(check_res);
- resolver_ref(res);
- data->check_res = check_res;
- data->res = res;
+ resolver_ref(resolver_to_check);
 
- if (resolve(check_res, ".", LDNS_RR_TYPE_NS, LDNS_RR_CLASS_IN,
-    data, check_resolver_done) != 0) {
+ if (resolve(res, ".", LDNS_RR_TYPE_NS, LDNS_RR_CLASS_IN,
+    resolver_to_check, check_resolver_done) != 0) {
  log_debug("check oppdot failed");
  /* do not overwrite normal DNS state, it might work */
- resolver_unref(check_res);
- resolver_unref(res);
+ resolver_unref(resolver_to_check);
 
- res->check_tv.tv_sec = RESOLVER_CHECK_SEC;
- evtimer_add(&res->check_ev, &res->check_tv);
+ resolver_to_check->check_tv.tv_sec = RESOLVER_CHECK_SEC;
+ evtimer_add(&resolver_to_check->check_ev,
+    &resolver_to_check->check_tv);
 
  log_debug("%s: evtimer_add: %lld - %s: %s", __func__,
-    data->res->check_tv.tv_sec,
-    uw_resolver_type_str[data->res->type],
-    uw_resolver_state_str[data->res->state]);
+    resolver_to_check->check_tv.tv_sec,
+    uw_resolver_type_str[resolver_to_check->type],
+    uw_resolver_state_str[resolver_to_check->state]);
  }
 }
 
 void
-check_resolver_done(void *arg, int rcode, void *answer_packet, int answer_len,
-    int sec, char *why_bogus)
+check_resolver_done(struct uw_resolver *res, void *arg, int rcode,
+    void *answer_packet, int answer_len, int sec, char *why_bogus)
 {
- struct check_resolver_data *data;
- struct timeval tv = {0, 1};
- enum uw_resolver_state prev_state;
- char *str;
-
- data = (struct check_resolver_data *)arg;
+ struct uw_resolver *checked_resolver = arg;
+ struct timeval tv = {0, 1};
+ enum uw_resolver_state prev_state;
+ char *str;
 
  log_debug("%s: %s rcode: %d", __func__,
-    uw_resolver_type_str[data->res->type], rcode);
+    uw_resolver_type_str[checked_resolver->type], rcode);
 
- prev_state = data->res->state;
+ prev_state = checked_resolver->state;
 
  if (answer_len < LDNS_HEADER_SIZE) {
- data->res->state = DEAD;
+ checked_resolver->state = DEAD;
  log_warnx("bad packet: too short");
  goto out;
  }
 
  if (rcode == LDNS_RCODE_SERVFAIL) {
- if (data->check_res->oppdot == data->res->oppdot) {
- data->res->state = DEAD;
- if (data->res->oppdot) {
+ if (res->oppdot == checked_resolver->oppdot) {
+ checked_resolver->state = DEAD;
+ if (checked_resolver->oppdot) {
  /* downgrade from opportunistic DoT */
- switch (data->res->type) {
+ switch (checked_resolver->type) {
  case UW_RES_DHCP:
  new_forwarders(0);
  break;
@@ -1362,9 +1345,9 @@ check_resolver_done(void *arg, int rcode, void *answer_packet, int answer_len,
  goto out;
  }
 
- if (data->check_res->oppdot && !data->res->oppdot) {
+ if (res->oppdot && !checked_resolver->oppdot) {
  /* upgrade to opportunistic DoT */
- switch (data->res->type) {
+ switch (checked_resolver->type) {
  case UW_RES_DHCP:
  new_forwarders(1);
  break;
@@ -1382,56 +1365,57 @@ check_resolver_done(void *arg, int rcode, void *answer_packet, int answer_len,
  }
 
  if (sec == SECURE) {
- data->res->state = VALIDATING;
+ checked_resolver->state = VALIDATING;
  if (!(evtimer_pending(&trust_anchor_timer, NULL)))
  evtimer_add(&trust_anchor_timer, &tv);
  } else if (rcode == LDNS_RCODE_NOERROR &&
     LDNS_RCODE_WIRE((uint8_t*)answer_packet) == LDNS_RCODE_NOERROR) {
  log_debug("%s: why bogus: %s", __func__, why_bogus);
- data->res->state = RESOLVING;
+ checked_resolver->state = RESOLVING;
  /* best effort */
- data->res->why_bogus = strdup(why_bogus);
+ checked_resolver->why_bogus = strdup(why_bogus);
  } else
- data->res->state = DEAD; /* we know the root exists */
+ checked_resolver->state = DEAD; /* we know the root exists */
 
 out:
- if (!data->res->stop && data->res->state == DEAD) {
+ if (!checked_resolver->stop && checked_resolver->state == DEAD) {
  if (prev_state == DEAD)
- data->res->check_tv.tv_sec *= 2;
+ checked_resolver->check_tv.tv_sec *= 2;
  else
- data->res->check_tv.tv_sec = RESOLVER_CHECK_SEC;
+ checked_resolver->check_tv.tv_sec = RESOLVER_CHECK_SEC;
 
- if (data->res->check_tv.tv_sec > RESOLVER_CHECK_MAXSEC)
- data->res->check_tv.tv_sec = RESOLVER_CHECK_MAXSEC;
+ if (checked_resolver->check_tv.tv_sec > RESOLVER_CHECK_MAXSEC)
+ checked_resolver->check_tv.tv_sec =
+    RESOLVER_CHECK_MAXSEC;
 
- evtimer_add(&data->res->check_ev, &data->res->check_tv);
+ evtimer_add(&checked_resolver->check_ev,
+    &checked_resolver->check_tv);
 
  log_debug("%s: evtimer_add: %lld - %s: %s", __func__,
-    data->res->check_tv.tv_sec,
-    uw_resolver_type_str[data->res->type],
-    uw_resolver_state_str[data->res->state]);
+    checked_resolver->check_tv.tv_sec,
+    uw_resolver_type_str[checked_resolver->type],
+    uw_resolver_state_str[checked_resolver->state]);
  }
 
  log_debug("%s: %s: %s", __func__,
-    uw_resolver_type_str[data->res->type],
-    uw_resolver_state_str[data->res->state]);
-
- log_debug("%s: %p - %p", __func__, data->res, data->res->ctx);
+    uw_resolver_type_str[checked_resolver->type],
+    uw_resolver_state_str[checked_resolver->state]);
 
- resolver_unref(data->res);
- data->check_res->stop = 1; /* do not free in callback */
- resolver_unref(data->check_res);
+ log_debug("%s: %p - %p", __func__, checked_resolver,
+    checked_resolver->ctx);
 
- free(data);
+ resolver_unref(checked_resolver);
+ res->stop = 1; /* do not free in callback */
 }
 
 void
 asr_resolve_done(struct asr_result *ar, void *arg)
 {
  struct resolver_cb_data *cb_data = arg;
- cb_data->cb(cb_data->data, ar->ar_rcode, ar->ar_data, ar->ar_datalen, 0,
-    "");
+ cb_data->cb(cb_data->res, cb_data->data, ar->ar_rcode, ar->ar_data,
+    ar->ar_datalen, 0, "");
  free(ar->ar_data);
+ resolver_unref(cb_data->res);
  free(cb_data);
 }
 
@@ -1440,8 +1424,9 @@ ub_resolve_done(void *arg, int rcode, void *answer_packet, int answer_len,
     int sec, char *why_bogus, int was_ratelimited)
 {
  struct resolver_cb_data *cb_data = arg;
- cb_data->cb(cb_data->data, rcode, answer_packet, answer_len, sec,
-    why_bogus);
+ cb_data->cb(cb_data->res, cb_data->data, rcode, answer_packet,
+    answer_len, sec, why_bogus);
+ resolver_unref(cb_data->res);
  free(cb_data);
 }
 
@@ -1779,13 +1764,10 @@ trust_anchor_resolve(void)
  if (res == NULL || res->state < VALIDATING)
  goto err;
 
- resolver_ref(res);
-
- if (resolve(res, ".",  LDNS_RR_TYPE_DNSKEY, LDNS_RR_CLASS_IN, res,
-    trust_anchor_resolve_done) != 0) {
- resolver_unref(res);
+ if (resolve(res, ".",  LDNS_RR_TYPE_DNSKEY, LDNS_RR_CLASS_IN, NULL,
+    trust_anchor_resolve_done) != 0)
  goto err;
- }
+
  return;
  err:
  evtimer_add(&trust_anchor_timer, &tv);
@@ -1798,10 +1780,9 @@ trust_anchor_timo(int fd, short events, void *arg)
 }
 
 void
-trust_anchor_resolve_done(void *arg, int rcode, void *answer_packet,
-    int answer_len, int sec, char *why_bogus)
+trust_anchor_resolve_done(struct uw_resolver *res, void *arg, int rcode,
+    void *answer_packet, int answer_len, int sec, char *why_bogus)
 {
- struct uw_resolver *res;
  struct ub_result *result;
  sldns_buffer *buf;
  struct regional *region;
@@ -1810,8 +1791,6 @@ trust_anchor_resolve_done(void *arg, int rcode, void *answer_packet,
  uint16_t dnskey_flags;
  char *str, rdata_buf[1024], *ta;
 
- res = (struct uw_resolver *)arg;
-
  if ((result = calloc(1, sizeof(*result))) == NULL)
  goto out;
 
@@ -1888,7 +1867,6 @@ trust_anchor_resolve_done(void *arg, int rcode, void *answer_packet,
  }
 out:
  ub_resolve_free(result);
- resolver_unref(res);
  evtimer_add(&trust_anchor_timer, &tv);
 }
 
diff --git unwind.h unwind.h
index 9e762681ac7..c3bafc4445e 100644
--- unwind.h
+++ unwind.h
@@ -153,7 +153,6 @@ struct query_imsg {
  int c;
  int err;
  int bogus;
- void *resolver;
  struct timespec tp;
 };
 
 

--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: unwind(8): refactor & simplify refcounting

Otto Moerbeek
On Tue, Nov 12, 2019 at 05:45:38PM +0100, Florian Obser wrote:

> Did I get this right? I'd appreciate it if someone could give this a
> once over.
>
>     Since resolve() switched to a callback mechanism all uw_resolver objects
>     pass through resolve() and either asr_resolve_done() or
>     ub_resolve_done().
>     With that we can pull resolver_ref() and resolver_unref() into those
>     functions to make the reference counting easier.
>     Only check_resolver is special since it needs to refcount the to be
>     checked resolver. But the resolver doing the actual work is
>     automatically refcounted by resolve() and *_resolve_done().
>     One last piece of the puzzle is to track the uw_resolver object in
>     cb_data so that the *_resolve_done() functions have access to it.
>     This also allowes us to remove the ad-hoc passing of the resolver in
>     query_imsg. Since the callback functions all need access to the
>     resolver that did the work we pass it in as first argument.
>

Reviewed the code, did some tests and it all looks good.

One nit: I would have declared a typedef for the callback funtion type
to be used in the struct resolver_cb_data and the prototype and the
definition of resolve(), it makes those lines easier to read.  But ok
anyway,

        -Otto


> diff --git resolver.c resolver.c
> index d1dce2dec71..2b7d81d29fc 100644
> --- resolver.c
> +++ resolver.c
> @@ -92,14 +92,11 @@ struct uw_resolver {
>   int64_t histogram[nitems(histogram_limits)];
>  };
>  
> -struct check_resolver_data {
> - struct uw_resolver *res;
> - struct uw_resolver *check_res;
> -};
> -
>  struct resolver_cb_data {
> - void (*cb)(void *, int, void *, int, int, char *);
> - void *data;
> + void (*cb)(struct uw_resolver *, void *, int, void *,
> +        int, int, char *);
> + void *data;
> + struct uw_resolver *res;
>  };
>  
>  __dead void resolver_shutdown(void);
> @@ -108,9 +105,10 @@ void resolver_dispatch_frontend(int, short, void *);
>  void resolver_dispatch_captiveportal(int, short, void *);
>  void resolver_dispatch_main(int, short, void *);
>  int resolve(struct uw_resolver *, const char*, int, int,
> -     void*, void (*cb)(void *, int, void *, int, int,
> -     char *));
> -void resolve_done(void *, int, void *, int, int, char *);
> +     void*, void (*cb)(struct uw_resolver *, void *,
> +     int, void *, int, int, char *));
> +void resolve_done(struct uw_resolver *, void *, int, void *,
> +     int, int, char *);
>  void ub_resolve_done(void *, int, void *, int, int, char *,
>       int);
>  void asr_resolve_done(struct asr_result *, void *);
> @@ -129,8 +127,8 @@ void set_forwarders_oppdot(struct uw_resolver *,
>  void resolver_check_timo(int, short, void *);
>  void resolver_free_timo(int, short, void *);
>  void check_resolver(struct uw_resolver *);
> -void check_resolver_done(void *, int, void *, int, int,
> -     char *);
> +void check_resolver_done(struct uw_resolver *, void *, int,
> +     void *, int, int, char *);
>  void schedule_recheck_all_resolvers(void);
>  int check_forwarders_changed(struct uw_forwarder_head *,
>       struct uw_forwarder_head *);
> @@ -154,8 +152,8 @@ int check_captive_portal_changed(struct uw_conf *,
>       struct uw_conf *);
>  void trust_anchor_resolve(void);
>  void trust_anchor_timo(int, short, void *);
> -void trust_anchor_resolve_done(void *, int, void *, int,
> -     int, char *);
> +void trust_anchor_resolve_done(struct uw_resolver *, void *,
> +     int, void *, int, int, char *);
>  void add_autoconf_forwarders(struct imsg_rdns_proposal *);
>  void rem_autoconf_forwarders(struct imsg_rdns_proposal *);
>  struct uw_forwarder *find_forwarder(struct uw_forwarder_head *,
> @@ -480,14 +478,10 @@ resolver_dispatch_frontend(int fd, short event, void *bula)
>   log_debug("%s: choosing %s", __func__,
>      uw_resolver_type_str[res->type]);
>  
> - query_imsg->resolver = res;
> - resolver_ref(res);
> -
>   clock_gettime(CLOCK_MONOTONIC, &query_imsg->tp);
>  
> - if ((resolve(res, query_imsg->qname, query_imsg->t,
> -    query_imsg->c, query_imsg, resolve_done)) != 0)
> - resolver_unref(res);
> + resolve(res, query_imsg->qname, query_imsg->t,
> +    query_imsg->c, query_imsg, resolve_done);
>   break;
>   case IMSG_FORWARDER:
>   /* make sure this is a string */
> @@ -773,16 +767,20 @@ resolver_dispatch_main(int fd, short event, void *bula)
>  
>  int
>  resolve(struct uw_resolver *res, const char* name, int rrtype, int rrclass,
> -    void *mydata, void (*cb)(void *, int, void *, int, int, char *))
> +    void *mydata, void (*cb)(struct uw_resolver *, void *, int, void *, int,
> +    int, char *))
>  {
>   struct resolver_cb_data *cb_data = NULL;
>   struct asr_query *aq = NULL;
>   int err;
>  
> + resolver_ref(res);
> +
>   if ((cb_data = malloc(sizeof(*cb_data))) == NULL)
>   goto err;
>   cb_data->cb = cb;
>   cb_data->data = mydata;
> + cb_data->res = res;
>  
>   switch(res->type) {
>   case UW_RES_ASR:
> @@ -816,15 +814,15 @@ resolve(struct uw_resolver *res, const char* name, int rrtype, int rrclass,
>   err:
>   free(cb_data);
>   free(aq);
> + resolver_unref(res);
>   return 1;
>  }
>  
>  void
> -resolve_done(void *arg, int rcode, void *answer_packet, int answer_len,
> -    int sec, char *why_bogus)
> +resolve_done(struct uw_resolver *res, void *arg, int rcode,
> +    void *answer_packet, int answer_len, int sec, char *why_bogus)
>  {
>   struct query_imsg *query_imsg;
> - struct uw_resolver *res;
>   struct timespec tp, elapsed;
>   int64_t ms;
>   size_t i;
> @@ -833,7 +831,6 @@ resolve_done(void *arg, int rcode, void *answer_packet, int answer_len,
>   clock_gettime(CLOCK_MONOTONIC, &tp);
>  
>   query_imsg = (struct query_imsg *)arg;
> - res = (struct uw_resolver *)query_imsg->resolver;
>  
>   timespecsub(&tp, &query_imsg->tp, &elapsed);
>  
> @@ -886,7 +883,6 @@ resolve_done(void *arg, int rcode, void *answer_packet, int answer_len,
>      answer_packet, answer_len);
>  
>   free(query_imsg);
> - resolver_unref(res);
>   return;
>  
>  servfail:
> @@ -894,7 +890,6 @@ servfail:
>   resolver_imsg_compose_frontend(IMSG_ANSWER_HEADER, 0, query_imsg,
>      sizeof(*query_imsg));
>   free(query_imsg);
> - resolver_unref(res);
>  }
>  
>  void
> @@ -1259,95 +1254,83 @@ resolver_free_timo(int fd, short events, void *arg)
>  }
>  
>  void
> -check_resolver(struct uw_resolver *res)
> +check_resolver(struct uw_resolver *resolver_to_check)
>  {
> - struct uw_resolver *check_res;
> - struct check_resolver_data *data;
> + struct uw_resolver *res;
>  
>   log_debug("%s: create_resolver", __func__);
> - if ((check_res = create_resolver(res->type, 0)) == NULL)
> - fatal("%s", __func__);
> - if ((data = malloc(sizeof(*data))) == NULL)
> + if ((res = create_resolver(resolver_to_check->type, 0)) == NULL)
>   fatal("%s", __func__);
>  
> - resolver_ref(check_res);
> - resolver_ref(res);
> - data->check_res = check_res;
> - data->res = res;
> + resolver_ref(resolver_to_check);
>  
> - if (resolve(check_res, ".", LDNS_RR_TYPE_NS, LDNS_RR_CLASS_IN,
> -    data, check_resolver_done) != 0) {
> - res->state = UNKNOWN;
> - resolver_unref(check_res);
> - resolver_unref(res);
> - res->check_tv.tv_sec = RESOLVER_CHECK_SEC;
> - evtimer_add(&res->check_ev, &res->check_tv);
> + if (resolve(res, ".", LDNS_RR_TYPE_NS, LDNS_RR_CLASS_IN,
> +    resolver_to_check, check_resolver_done) != 0) {
> + resolver_to_check->state = UNKNOWN;
> + resolver_unref(resolver_to_check);
> + resolver_to_check->check_tv.tv_sec = RESOLVER_CHECK_SEC;
> + evtimer_add(&resolver_to_check->check_ev,
> +    &resolver_to_check->check_tv);
>  
>   log_debug("%s: evtimer_add: %lld - %s: %s", __func__,
> -    data->res->check_tv.tv_sec,
> -    uw_resolver_type_str[data->res->type],
> -    uw_resolver_state_str[data->res->state]);
> +    resolver_to_check->check_tv.tv_sec,
> +    uw_resolver_type_str[resolver_to_check->type],
> +    uw_resolver_state_str[resolver_to_check->state]);
>   }
>  
> - if (!(res->type == UW_RES_DHCP || res->type == UW_RES_FORWARDER))
> + if (!(resolver_to_check->type == UW_RES_DHCP ||
> +    resolver_to_check->type == UW_RES_FORWARDER))
>   return;
>  
>   log_debug("%s: create_resolver for oppdot", __func__);
> - if ((check_res = create_resolver(res->type, 1)) == NULL)
> - fatal("%s", __func__);
> - if ((data = malloc(sizeof(*data))) == NULL)
> + if ((res = create_resolver(resolver_to_check->type, 1)) == NULL)
>   fatal("%s", __func__);
>  
> - resolver_ref(check_res);
> - resolver_ref(res);
> - data->check_res = check_res;
> - data->res = res;
> + resolver_ref(resolver_to_check);
>  
> - if (resolve(check_res, ".", LDNS_RR_TYPE_NS, LDNS_RR_CLASS_IN,
> -    data, check_resolver_done) != 0) {
> + if (resolve(res, ".", LDNS_RR_TYPE_NS, LDNS_RR_CLASS_IN,
> +    resolver_to_check, check_resolver_done) != 0) {
>   log_debug("check oppdot failed");
>   /* do not overwrite normal DNS state, it might work */
> - resolver_unref(check_res);
> - resolver_unref(res);
> + resolver_unref(resolver_to_check);
>  
> - res->check_tv.tv_sec = RESOLVER_CHECK_SEC;
> - evtimer_add(&res->check_ev, &res->check_tv);
> + resolver_to_check->check_tv.tv_sec = RESOLVER_CHECK_SEC;
> + evtimer_add(&resolver_to_check->check_ev,
> +    &resolver_to_check->check_tv);
>  
>   log_debug("%s: evtimer_add: %lld - %s: %s", __func__,
> -    data->res->check_tv.tv_sec,
> -    uw_resolver_type_str[data->res->type],
> -    uw_resolver_state_str[data->res->state]);
> +    resolver_to_check->check_tv.tv_sec,
> +    uw_resolver_type_str[resolver_to_check->type],
> +    uw_resolver_state_str[resolver_to_check->state]);
>   }
>  }
>  
>  void
> -check_resolver_done(void *arg, int rcode, void *answer_packet, int answer_len,
> -    int sec, char *why_bogus)
> +check_resolver_done(struct uw_resolver *res, void *arg, int rcode,
> +    void *answer_packet, int answer_len, int sec, char *why_bogus)
>  {
> - struct check_resolver_data *data;
> - struct timeval tv = {0, 1};
> - enum uw_resolver_state prev_state;
> - char *str;
> -
> - data = (struct check_resolver_data *)arg;
> + struct uw_resolver *checked_resolver = arg;
> + struct timeval tv = {0, 1};
> + enum uw_resolver_state prev_state;
> + char *str;
>  
>   log_debug("%s: %s rcode: %d", __func__,
> -    uw_resolver_type_str[data->res->type], rcode);
> +    uw_resolver_type_str[checked_resolver->type], rcode);
>  
> - prev_state = data->res->state;
> + prev_state = checked_resolver->state;
>  
>   if (answer_len < LDNS_HEADER_SIZE) {
> - data->res->state = DEAD;
> + checked_resolver->state = DEAD;
>   log_warnx("bad packet: too short");
>   goto out;
>   }
>  
>   if (rcode == LDNS_RCODE_SERVFAIL) {
> - if (data->check_res->oppdot == data->res->oppdot) {
> - data->res->state = DEAD;
> - if (data->res->oppdot) {
> + if (res->oppdot == checked_resolver->oppdot) {
> + checked_resolver->state = DEAD;
> + if (checked_resolver->oppdot) {
>   /* downgrade from opportunistic DoT */
> - switch (data->res->type) {
> + switch (checked_resolver->type) {
>   case UW_RES_DHCP:
>   new_forwarders(0);
>   break;
> @@ -1362,9 +1345,9 @@ check_resolver_done(void *arg, int rcode, void *answer_packet, int answer_len,
>   goto out;
>   }
>  
> - if (data->check_res->oppdot && !data->res->oppdot) {
> + if (res->oppdot && !checked_resolver->oppdot) {
>   /* upgrade to opportunistic DoT */
> - switch (data->res->type) {
> + switch (checked_resolver->type) {
>   case UW_RES_DHCP:
>   new_forwarders(1);
>   break;
> @@ -1382,56 +1365,57 @@ check_resolver_done(void *arg, int rcode, void *answer_packet, int answer_len,
>   }
>  
>   if (sec == SECURE) {
> - data->res->state = VALIDATING;
> + checked_resolver->state = VALIDATING;
>   if (!(evtimer_pending(&trust_anchor_timer, NULL)))
>   evtimer_add(&trust_anchor_timer, &tv);
>   } else if (rcode == LDNS_RCODE_NOERROR &&
>      LDNS_RCODE_WIRE((uint8_t*)answer_packet) == LDNS_RCODE_NOERROR) {
>   log_debug("%s: why bogus: %s", __func__, why_bogus);
> - data->res->state = RESOLVING;
> + checked_resolver->state = RESOLVING;
>   /* best effort */
> - data->res->why_bogus = strdup(why_bogus);
> + checked_resolver->why_bogus = strdup(why_bogus);
>   } else
> - data->res->state = DEAD; /* we know the root exists */
> + checked_resolver->state = DEAD; /* we know the root exists */
>  
>  out:
> - if (!data->res->stop && data->res->state == DEAD) {
> + if (!checked_resolver->stop && checked_resolver->state == DEAD) {
>   if (prev_state == DEAD)
> - data->res->check_tv.tv_sec *= 2;
> + checked_resolver->check_tv.tv_sec *= 2;
>   else
> - data->res->check_tv.tv_sec = RESOLVER_CHECK_SEC;
> + checked_resolver->check_tv.tv_sec = RESOLVER_CHECK_SEC;
>  
> - if (data->res->check_tv.tv_sec > RESOLVER_CHECK_MAXSEC)
> - data->res->check_tv.tv_sec = RESOLVER_CHECK_MAXSEC;
> + if (checked_resolver->check_tv.tv_sec > RESOLVER_CHECK_MAXSEC)
> + checked_resolver->check_tv.tv_sec =
> +    RESOLVER_CHECK_MAXSEC;
>  
> - evtimer_add(&data->res->check_ev, &data->res->check_tv);
> + evtimer_add(&checked_resolver->check_ev,
> +    &checked_resolver->check_tv);
>  
>   log_debug("%s: evtimer_add: %lld - %s: %s", __func__,
> -    data->res->check_tv.tv_sec,
> -    uw_resolver_type_str[data->res->type],
> -    uw_resolver_state_str[data->res->state]);
> +    checked_resolver->check_tv.tv_sec,
> +    uw_resolver_type_str[checked_resolver->type],
> +    uw_resolver_state_str[checked_resolver->state]);
>   }
>  
>   log_debug("%s: %s: %s", __func__,
> -    uw_resolver_type_str[data->res->type],
> -    uw_resolver_state_str[data->res->state]);
> -
> - log_debug("%s: %p - %p", __func__, data->res, data->res->ctx);
> +    uw_resolver_type_str[checked_resolver->type],
> +    uw_resolver_state_str[checked_resolver->state]);
>  
> - resolver_unref(data->res);
> - data->check_res->stop = 1; /* do not free in callback */
> - resolver_unref(data->check_res);
> + log_debug("%s: %p - %p", __func__, checked_resolver,
> +    checked_resolver->ctx);
>  
> - free(data);
> + resolver_unref(checked_resolver);
> + res->stop = 1; /* do not free in callback */
>  }
>  
>  void
>  asr_resolve_done(struct asr_result *ar, void *arg)
>  {
>   struct resolver_cb_data *cb_data = arg;
> - cb_data->cb(cb_data->data, ar->ar_rcode, ar->ar_data, ar->ar_datalen, 0,
> -    "");
> + cb_data->cb(cb_data->res, cb_data->data, ar->ar_rcode, ar->ar_data,
> +    ar->ar_datalen, 0, "");
>   free(ar->ar_data);
> + resolver_unref(cb_data->res);
>   free(cb_data);
>  }
>  
> @@ -1440,8 +1424,9 @@ ub_resolve_done(void *arg, int rcode, void *answer_packet, int answer_len,
>      int sec, char *why_bogus, int was_ratelimited)
>  {
>   struct resolver_cb_data *cb_data = arg;
> - cb_data->cb(cb_data->data, rcode, answer_packet, answer_len, sec,
> -    why_bogus);
> + cb_data->cb(cb_data->res, cb_data->data, rcode, answer_packet,
> +    answer_len, sec, why_bogus);
> + resolver_unref(cb_data->res);
>   free(cb_data);
>  }
>  
> @@ -1779,13 +1764,10 @@ trust_anchor_resolve(void)
>   if (res == NULL || res->state < VALIDATING)
>   goto err;
>  
> - resolver_ref(res);
> -
> - if (resolve(res, ".",  LDNS_RR_TYPE_DNSKEY, LDNS_RR_CLASS_IN, res,
> -    trust_anchor_resolve_done) != 0) {
> - resolver_unref(res);
> + if (resolve(res, ".",  LDNS_RR_TYPE_DNSKEY, LDNS_RR_CLASS_IN, NULL,
> +    trust_anchor_resolve_done) != 0)
>   goto err;
> - }
> +
>   return;
>   err:
>   evtimer_add(&trust_anchor_timer, &tv);
> @@ -1798,10 +1780,9 @@ trust_anchor_timo(int fd, short events, void *arg)
>  }
>  
>  void
> -trust_anchor_resolve_done(void *arg, int rcode, void *answer_packet,
> -    int answer_len, int sec, char *why_bogus)
> +trust_anchor_resolve_done(struct uw_resolver *res, void *arg, int rcode,
> +    void *answer_packet, int answer_len, int sec, char *why_bogus)
>  {
> - struct uw_resolver *res;
>   struct ub_result *result;
>   sldns_buffer *buf;
>   struct regional *region;
> @@ -1810,8 +1791,6 @@ trust_anchor_resolve_done(void *arg, int rcode, void *answer_packet,
>   uint16_t dnskey_flags;
>   char *str, rdata_buf[1024], *ta;
>  
> - res = (struct uw_resolver *)arg;
> -
>   if ((result = calloc(1, sizeof(*result))) == NULL)
>   goto out;
>  
> @@ -1888,7 +1867,6 @@ trust_anchor_resolve_done(void *arg, int rcode, void *answer_packet,
>   }
>  out:
>   ub_resolve_free(result);
> - resolver_unref(res);
>   evtimer_add(&trust_anchor_timer, &tv);
>  }
>  
> diff --git unwind.h unwind.h
> index 9e762681ac7..c3bafc4445e 100644
> --- unwind.h
> +++ unwind.h
> @@ -153,7 +153,6 @@ struct query_imsg {
>   int c;
>   int err;
>   int bogus;
> - void *resolver;
>   struct timespec tp;
>  };
>  
>  
>
> --
> I'm not entirely sure you are real.
>