- Fix that enables modules to register twice for the same

serviced_query, without race conditions or administration issues.
  This should not happen with the current codebase, but it is robust.


git-svn-id: file:///svn/unbound/trunk@2730 be551aaa-1e26-0410-a405-d3ace91eadb9

daemon/worker.c

@@ -1243,17 +1243,6 @@ worker_delete(struct worker* worker)
 	free(worker);
 }
 
-/** compare outbound entry qstates */
-static int
-outbound_entry_compare(void* a, void* b)
-{
-	struct outbound_entry* e1 = (struct outbound_entry*)a;
-	struct outbound_entry* e2 = (struct outbound_entry*)b;
-	if(e1->qstate == e2->qstate)
-		return 1;
-	return 0;
-}
-
 struct outbound_entry*
 worker_send_query(uint8_t* qname, size_t qnamelen, uint16_t qtype,
 	uint16_t qclass, uint16_t flags, int dnssec, int want_dnssec,
@@ -1270,7 +1259,7 @@ worker_send_query(uint8_t* qname, size_t qnamelen, uint16_t qtype,
 		qnamelen, qtype, qclass, flags, dnssec, want_dnssec,
 		q->env->cfg->tcp_upstream, q->env->cfg->ssl_upstream, addr,
 		addrlen, zone, zonelen, worker_handle_service_reply, e,
-		worker->back->udp_buff, &outbound_entry_compare);
+		worker->back->udp_buff);
 	if(!e->qsent) {
 		return NULL;
 	}

doc/Changelog

@@ -1,5 +1,8 @@
 31 July 2012: Wouter
 	- Improved forward-first and stub-first documentation.
+	- Fix that enables modules to register twice for the same
+	  serviced_query, without race conditions or administration issues.
+	  This should not happen with the current codebase, but it is robust.
 
 30 July 2012: Wouter
 	- tag 1.4.18rc2.

libunbound/libworker.c

@@ -705,17 +705,6 @@ void libworker_alloc_cleanup(void* arg)
         slabhash_clear(w->env->msg_cache);
 }
 
-/** compare outbound entry qstates */
-static int
-outbound_entry_compare(void* a, void* b)
-{
-        struct outbound_entry* e1 = (struct outbound_entry*)a;
-        struct outbound_entry* e2 = (struct outbound_entry*)b;
-        if(e1->qstate == e2->qstate)
-                return 1;
-        return 0;
-}
-
 struct outbound_entry* libworker_send_query(uint8_t* qname, size_t qnamelen,
         uint16_t qtype, uint16_t qclass, uint16_t flags, int dnssec,
 	int want_dnssec, struct sockaddr_storage* addr, socklen_t addrlen,
@@ -731,7 +720,7 @@ struct outbound_entry* libworker_send_query(uint8_t* qname, size_t qnamelen,
 		qnamelen, qtype, qclass, flags, dnssec, want_dnssec,
 		q->env->cfg->tcp_upstream, q->env->cfg->ssl_upstream, addr,
 		addrlen, zone, zonelen, libworker_handle_service_reply, e,
-		w->back->udp_buff, &outbound_entry_compare);
+		w->back->udp_buff);
 	if(!e->qsent) {
 		return NULL;
 	}

services/outside_network.c

@@ -1441,7 +1441,7 @@ static void
 serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c,
 	struct comm_reply* rep)
 {
-	struct service_callback* p = sq->cblist, *n;
+	struct service_callback* p;
 	int dobackup = (sq->cblist && sq->cblist->next); /* >1 cb*/
 	uint8_t *backup_p = NULL;
 	size_t backlen = 0;
@@ -1500,8 +1500,9 @@ serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c,
 		}
 		sq->outnet->svcd_overhead = backlen;
 	}
-	while(p) {
-		n = p->next;
+	/* test the actual sq->cblist, because the next elem could be deleted*/
+	while((p=sq->cblist) != NULL) {
+		sq->cblist = p->next; /* remove this element */
 		if(dobackup && c) {
 			ldns_buffer_clear(c->buffer);
 			ldns_buffer_write(c->buffer, backup_p, backlen);
@@ -1509,7 +1510,7 @@ serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c,
 		}
 		fptr_ok(fptr_whitelist_serviced_query(p->cb));
 		(void)(*p->cb)(c, p->cb_arg, error, rep);
-		p = n;
+		free(p);
 	}
 	if(backup_p) {
 		free(backup_p);
@@ -1783,37 +1784,21 @@ serviced_udp_callback(struct comm_point* c, void* arg, int error,
 	return 0;
 }
 
-/** find callback in list */
-static struct service_callback*
-callback_list_find(struct serviced_query* sq, void* cb_arg, 
-	int (*arg_compare)(void*,void*))
-{
-	struct service_callback* p;
-	for(p = sq->cblist; p; p = p->next) {
-		if(arg_compare(p->cb_arg, cb_arg))
-			return p;
-	}
-	return NULL;
-}
-
 struct serviced_query* 
 outnet_serviced_query(struct outside_network* outnet,
 	uint8_t* qname, size_t qnamelen, uint16_t qtype, uint16_t qclass,
 	uint16_t flags, int dnssec, int want_dnssec, int tcp_upstream,
 	int ssl_upstream, struct sockaddr_storage* addr, socklen_t addrlen,
 	uint8_t* zone, size_t zonelen, comm_point_callback_t* callback,
-	void* callback_arg, ldns_buffer* buff, int (*arg_compare)(void*,void*))
+	void* callback_arg, ldns_buffer* buff)
 {
 	struct serviced_query* sq;
 	struct service_callback* cb;
 	serviced_gen_query(buff, qname, qnamelen, qtype, qclass, flags);
 	sq = lookup_serviced(outnet, buff, dnssec, addr, addrlen);
-	if(sq) {
-		/* see if it is a duplicate notification request for cb_arg */
-		if(callback_list_find(sq, callback_arg, arg_compare)) {
-			return sq;
-		}
-	}
+	/* duplicate entries are inclded in the callback list, because
+	 * there is a counterpart registration by our caller that needs to
+	 * be doubly-removed (with callbacks perhaps). */
 	if(!(cb = (struct service_callback*)malloc(sizeof(*cb))))
 		return NULL;
 	if(!sq) {

services/outside_network.h

@@ -468,8 +468,6 @@ void pending_delete(struct outside_network* outnet, struct pending* p);
 	authoritative.
  * @param zonelen: length of zone.
  * @param buff: scratch buffer to create query contents in. Empty on exit.
- * @param arg_compare: function to compare callback args, return true if 
- * 	identical. It is given the callback_arg and args that are listed.
  * @return 0 on error, or pointer to serviced query that is used to answer
  *	this serviced query may be shared with other callbacks as well.
  */
@@ -478,8 +476,7 @@ struct serviced_query* outnet_serviced_query(struct outside_network* outnet,
 	uint16_t flags, int dnssec, int want_dnssec, int tcp_upstream,
 	int ssl_upstream, struct sockaddr_storage* addr, socklen_t addrlen,
 	uint8_t* zone, size_t zonelen, comm_point_callback_t* callback,
-	void* callback_arg, ldns_buffer* buff,
-	int (*arg_compare)(void*,void*));
+	void* callback_arg, ldns_buffer* buff);
 
 /**
  * Remove service query callback.

testcode/fake_event.c

@@ -1041,14 +1041,13 @@ struct serviced_query* outnet_serviced_query(struct outside_network* outnet,
 	int ATTR_UNUSED(tcp_upstream), int ATTR_UNUSED(ssl_upstream),
 	struct sockaddr_storage* addr, socklen_t addrlen, uint8_t* zone,
 	size_t zonelen, comm_point_callback_t* callback, void* callback_arg,
-	ldns_buffer* ATTR_UNUSED(buff), int (*arg_compare)(void*,void*))
+	ldns_buffer* ATTR_UNUSED(buff))
 {
 	struct replay_runtime* runtime = (struct replay_runtime*)outnet->base;
 	struct fake_pending* pend = (struct fake_pending*)calloc(1,
 		sizeof(struct fake_pending));
 	char z[256];
 	ldns_status status;
-	(void)arg_compare;
 	log_assert(pend);
 	log_nametypeclass(VERB_OPS, "pending serviced query", 
 		qname, qtype, qclass);