From: Wouter Wijngaards <wouter@nlnetlabs.nl>
Date: Tue, 6 Feb 2007 16:26:19 +0000 (+0000)
Subject: review of services, daemon and testcode.
X-Git-Tag: release-0.0~55
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d8cbd99dc10f20c2e4deba408c69de3469494777;p=thirdparty%2Funbound.git

review of services, daemon and testcode.


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

diff --git a/daemon/unbound.c b/daemon/unbound.c
index eec2b252a..e9dbd2e2b 100644
--- a/daemon/unbound.c
+++ b/daemon/unbound.c
@@ -51,15 +51,15 @@
 static void usage()
 {
 	printf("usage: unbound [options]\n");
-	printf("\tstart unbound daemon DNS resolver.\n");
+	printf("	start unbound daemon DNS resolver.\n");
 	printf("-h	this help\n");
 	printf("-p port	the port to listen on\n");
 	printf("-v	verbose (multiple times increase verbosity)\n");
 	printf("-f ip	set forwarder address\n");
 	printf("-z port	set forwarder port\n");
 	printf("Version %s\n", PACKAGE_VERSION);
-	printf("BSD licensed, see LICENSE file in source package.\n");
-	printf("Report bugs to %s.\n", PACKAGE_BUGREPORT);
+	printf("BSD licensed, see LICENSE in source package for details.\n");
+	printf("Report bugs to %s\n", PACKAGE_BUGREPORT);
 }
 
 /** getopt global, in case header files fail to declare it. */
@@ -71,6 +71,7 @@ extern char* optarg;
  * main program. Set options given commandline arguments.
  * @param argc: number of commandline arguments.
  * @param argv: array of commandline arguments.
+ * @return: exit status of the program.
  */
 int 
 main(int argc, char* argv[])
diff --git a/daemon/worker.c b/daemon/worker.c
index 0252adfd8..ecc04dfc2 100644
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -59,7 +59,8 @@
 #define ID_AND_FLAGS 4
 
 /** reply to query with given error code */
-static void replyerror(int r, struct worker* worker)
+static void 
+replyerror(int r, struct worker* worker)
 {
 	LDNS_QR_SET(ldns_buffer_begin(worker->query_reply.c->buffer));
 	LDNS_RCODE_SET(ldns_buffer_begin(worker->query_reply.c->buffer), r);
@@ -68,7 +69,8 @@ static void replyerror(int r, struct worker* worker)
 }
 
 /** process incoming replies from the network */
-static int worker_handle_reply(struct comm_point* c, void* arg, int error, 
+static int 
+worker_handle_reply(struct comm_point* c, void* arg, int error, 
 	struct comm_reply* ATTR_UNUSED(reply_info))
 {
 	struct worker* worker = (struct worker*)arg;
@@ -90,7 +92,8 @@ static int worker_handle_reply(struct comm_point* c, void* arg, int error,
 }
 
 /** process incoming request */
-static void worker_process_query(struct worker* worker) 
+static void 
+worker_process_query(struct worker* worker) 
 {
 	/* query the forwarding address */
 	pending_udp_query(worker->back, worker->query_reply.c->buffer, 
@@ -101,7 +104,8 @@ static void worker_process_query(struct worker* worker)
 /** check request sanity. Returns error code, 0 OK, or -1 discard. 
  * @param pkt: the wire packet to examine for sanity.
 */
-static int worker_check_request(ldns_buffer* pkt)
+static int 
+worker_check_request(ldns_buffer* pkt)
 {
 	if(ldns_buffer_limit(pkt) < LDNS_HEADER_SIZE) {
 		verbose(VERB_DETAIL, "request too short, discarded");
@@ -140,13 +144,14 @@ static int worker_check_request(ldns_buffer* pkt)
 }
 
 /** handles callbacks from listening event interface */
-static int worker_handle_request(struct comm_point* c, void* arg, int error,
+static int 
+worker_handle_request(struct comm_point* c, void* arg, int error,
 	struct comm_reply* repinfo)
 {
 	struct worker* worker = (struct worker*)arg;
 	int ret;
 	log_info("worker handle request");
-	if(error != 0) {
+	if(error != NETEVENT_NOERROR) {
 		log_err("called with err=%d", error);
 		return 0;
 	}
@@ -171,7 +176,8 @@ static int worker_handle_request(struct comm_point* c, void* arg, int error,
 }
 
 /** worker signal callback */
-void worker_sighandler(int sig, void* arg)
+void 
+worker_sighandler(int sig, void* arg)
 {
 	/* note that log, print, syscalls here give race conditions. */
 	struct worker* worker = (struct worker*)arg;
@@ -194,9 +200,9 @@ void worker_sighandler(int sig, void* arg)
 	}
 }
 
-struct worker* worker_init(const char* port, int do_ip4, int do_ip6,
-	int do_udp, int do_tcp, size_t buffer_size, size_t numports,
-	int base_port)
+struct worker* 
+worker_init(const char* port, int do_ip4, int do_ip6, int do_udp, int do_tcp, 
+	size_t buffer_size, size_t numports, int base_port)
 {
 	struct worker* worker = (struct worker*)calloc(1, 
 		sizeof(struct worker));
@@ -221,14 +227,15 @@ struct worker* worker_init(const char* port, int do_ip4, int do_ip6,
 		do_ip4, do_ip6, do_udp, do_tcp, buffer_size, 
 		worker_handle_request, worker);
 	if(!worker->front) {
-		worker_delete(worker);
 		log_err("could not create listening sockets");
+		worker_delete(worker);
 		return NULL;
 	}
 	worker->back = outside_network_create(worker->base,
 		buffer_size, numports, NULL, 0, do_ip4, do_ip6, base_port);
 	if(!worker->back) {
 		log_err("could not create outgoing sockets");
+		worker_delete(worker);
 		return NULL;
 	}
 	/* init random(), large table size. */
@@ -240,12 +247,14 @@ struct worker* worker_init(const char* port, int do_ip4, int do_ip6,
 	return worker;
 }
 
-void worker_work(struct worker* worker)
+void 
+worker_work(struct worker* worker)
 {
 	comm_base_dispatch(worker->base);
 }
 
-void worker_delete(struct worker* worker)
+void 
+worker_delete(struct worker* worker)
 {
 	if(!worker) 
 		return;
@@ -256,7 +265,8 @@ void worker_delete(struct worker* worker)
 	free(worker);
 }
 
-int worker_set_fwd(struct worker* worker, const char* ip, const char* port)
+int 
+worker_set_fwd(struct worker* worker, const char* ip, const char* port)
 {
 	uint16_t p;
 	log_assert(worker && ip);
diff --git a/daemon/worker.h b/daemon/worker.h
index c89885aa7..baaf756f2 100644
--- a/daemon/worker.h
+++ b/daemon/worker.h
@@ -58,30 +58,25 @@ struct outside_network;
 struct worker {
 	/** the event base this worker works with */
 	struct comm_base* base;
-
 	/** the frontside listening interface where request events come in */
 	struct listen_dnsport* front;
-
 	/** the backside outside network interface to the auth servers */
 	struct outside_network* back;
-
-	/** our one and only query, packet buffer and where to send. */
-	struct comm_reply query_reply;
+	/** the signal handler */
+	struct comm_signal *comsig;
 
 	/** number of requests currently active */
 	int num_requests;
-
-	/** random() table for this worker. */
-	char rndstate[RND_STATE_SIZE];
+	/** our one and only query, packet buffer and where to send. */
+	struct comm_reply query_reply;
 
 	/** address to forward to */
 	struct sockaddr_storage fwd_addr;
-
 	/** length of fwd_addr */
 	socklen_t fwd_addrlen;
 
-	/** the signal handler */
-	struct comm_signal *comsig;
+	/** random() table for this worker. */
+	char rndstate[RND_STATE_SIZE];
 };
 
 /**
diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c
index 04b423029..f28490400 100644
--- a/services/listen_dnsport.c
+++ b/services/listen_dnsport.c
@@ -53,7 +53,7 @@
 
 /** number of queued TCP connections for listen() */
 #define TCP_BACKLOG 5 
-/** number of simultaneous open TCP connections */
+/** number of simultaneous open TCP connections for queries */
 #define TCP_COUNT 10 
 
 /**
@@ -258,6 +258,7 @@ listen_create_if(const char* ifname, struct listen_dnsport* front,
 		if(!cp_tcp) {
 			log_err("can't create commpoint");	
 			comm_point_delete(cp_udp);
+			close(s);
 			return 0;
 		}
 	}
@@ -306,12 +307,10 @@ listen_create(struct comm_base* base, int num_ifs, const char* ifs[],
 	
 	/* getaddrinfo */
 	memset(&hints, 0, sizeof(hints));
-
 	hints.ai_flags = AI_PASSIVE;
 	/* no name lookups on our listening ports */
 	if(num_ifs > 0)
 		hints.ai_flags |= AI_NUMERICHOST;
-
 	hints.ai_family = AF_UNSPEC;
 	if(!do_ip4 && !do_ip6) {
 		listen_delete(front);
@@ -382,4 +381,3 @@ listen_delete(struct listen_dnsport* front)
 	ldns_buffer_free(front->udp_buff);
 	free(front);
 }
-
diff --git a/services/listen_dnsport.h b/services/listen_dnsport.h
index 44d919ad4..d07ac1eaf 100644
--- a/services/listen_dnsport.h
+++ b/services/listen_dnsport.h
@@ -102,7 +102,6 @@ struct listen_dnsport* listen_create(struct comm_base* base,
  */
 void listen_delete(struct listen_dnsport* listen);
 
-
 /**
  * Create and bind nonblocking UDP socket
  * @param addr: address info ready to make socket.
@@ -110,5 +109,4 @@ void listen_delete(struct listen_dnsport* listen);
  */
 int create_udp_sock(struct addrinfo* addr);
 
-
 #endif /* LISTEN_DNSPORT_H */
diff --git a/services/outside_network.c b/services/outside_network.c
index ea4255278..a93456c1a 100644
--- a/services/outside_network.c
+++ b/services/outside_network.c
@@ -56,7 +56,8 @@
 #define MAX_ID_RETRY 1000
 
 /** compare function of pending rbtree */
-static int pending_cmp(const void* key1, const void* key2)
+static int 
+pending_cmp(const void* key1, const void* key2)
 {
 	struct pending *p1 = (struct pending*)key1;
 	struct pending *p2 = (struct pending*)key2;
@@ -74,7 +75,8 @@ static int pending_cmp(const void* key1, const void* key2)
 }
 
 /** callback for incoming udp answers from the network. */
-static int outnet_udp_cb(struct comm_point* c, void* arg, int error,
+static int 
+outnet_udp_cb(struct comm_point* c, void* arg, int error,
 	struct comm_reply *reply_info)
 {
 	struct outside_network* outnet = (struct outside_network*)arg;
@@ -82,7 +84,7 @@ static int outnet_udp_cb(struct comm_point* c, void* arg, int error,
 	struct pending* p;
 	log_info("answer cb");
 
-	if(error != 0) {
+	if(error != NETEVENT_NOERROR) {
 		log_info("outnetudp got udp error %d", error);
 		return 0;
 	}
@@ -93,16 +95,17 @@ static int outnet_udp_cb(struct comm_point* c, void* arg, int error,
 	memcpy(&key.addr, &reply_info->addr, reply_info->addrlen);
 	key.addrlen = reply_info->addrlen;
 
-	/* find it, see if this thing is a valid query */
+	/* find it, see if this thing is a valid query response */
 	p = (struct pending*)rbtree_search(outnet->pending, &key);
 	if(!p) {
-		verbose(VERB_DETAIL, "received uncalled udp reply. dropped.");
+		verbose(VERB_DETAIL, "received unsolicited udp reply. dropped.");
 		return 0;
 	}
 
 	verbose(VERB_ALGO, "received udp reply.");
 	if(p->c != c) {
-		verbose(VERB_DETAIL, "received answer on wrong port. dropped");
+		verbose(VERB_DETAIL, "received reply id,addr on wrong port. "
+			"dropped.");
 		return 0;
 	}
 	comm_timer_disable(p->timer);
@@ -118,8 +121,8 @@ static int outnet_udp_cb(struct comm_point* c, void* arg, int error,
   * @param porthint: if not -1, it gives the port to base range on.
   * @return: file descriptor
   */
-static int open_udp_port_range(const char* ifname, struct addrinfo* hints,
-	int porthint)
+static int 
+open_udp_port_range(const char* ifname, struct addrinfo* hints, int porthint)
 {
 	struct addrinfo *res = NULL;
 	int r, s;
@@ -186,7 +189,8 @@ make_udp_range(struct comm_point** coms, const char* ifname,
 }
 
 /** returns true is string addr is an ip6 specced address. */
-int str_is_ip6(const char* str)
+int 
+str_is_ip6(const char* str)
 {
 	if(strchr(str, ':'))
 		return 1;
@@ -194,9 +198,9 @@ int str_is_ip6(const char* str)
 }
 
 /** calculate number of ip4 and ip6 interfaces, times multiplier. */
-static void calc_num46(const char** ifs, int num_ifs, 
-	int do_ip4, int do_ip6, size_t multiplier, 
-	size_t* num_ip4, size_t* num_ip6)
+static void 
+calc_num46(const char** ifs, int num_ifs, int do_ip4, int do_ip6, 
+	size_t multiplier, size_t* num_ip4, size_t* num_ip6)
 {
 	int i;
 	*num_ip4 = 0;
@@ -222,7 +226,8 @@ static void calc_num46(const char** ifs, int num_ifs,
 }
 
 /** callback for udp timeout */
-static void pending_udp_timer_cb(void *arg)
+static void 
+pending_udp_timer_cb(void *arg)
 {
 	struct pending* p = (struct pending*)arg;
 	/* it timed out */
@@ -245,6 +250,7 @@ outside_network_create(struct comm_base *base, size_t bufsize,
 	outnet->base = base;
 	calc_num46(ifs, num_ifs, do_ip4, do_ip6, num_ports, 
 		&outnet->num_udp4, &outnet->num_udp6);
+	/* adds +1 to portnums so we do not allocate zero bytes. */
 	if(	!(outnet->udp_buff = ldns_buffer_new(bufsize)) ||
 		!(outnet->udp4_ports = (struct comm_point **)calloc(
 			outnet->num_udp4+1, sizeof(struct comm_point*))) ||
@@ -297,7 +303,8 @@ outside_network_create(struct comm_base *base, size_t bufsize,
 	return outnet;
 }
 
-void outside_network_delete(struct outside_network* outnet)
+void 
+outside_network_delete(struct outside_network* outnet)
 {
 	if(!outnet)
 		return;
@@ -329,7 +336,8 @@ void outside_network_delete(struct outside_network* outnet)
 	free(outnet);
 }
 
-void pending_delete(struct outside_network* outnet, struct pending* p)
+void 
+pending_delete(struct outside_network* outnet, struct pending* p)
 {
 	if(!p)
 		return;
@@ -362,7 +370,9 @@ new_pending(struct outside_network* outnet, ldns_buffer* packet,
 		return NULL;
 	}
 	/* set */
-	pend->id = LDNS_ID_WIRE(ldns_buffer_begin(packet));
+	/* id uses lousy random() TODO use better and entropy */
+	pend->id = (random()>>8) & 0xffff;
+	LDNS_ID_SET(ldns_buffer_begin(packet), pend->id);
 	memcpy(&pend->addr, addr, addrlen);
 	pend->addrlen = addrlen;
 	pend->cb = callback;
@@ -389,7 +399,8 @@ new_pending(struct outside_network* outnet, ldns_buffer* packet,
  * @param addr: the sockaddr to examine.
  * return: true if sockaddr is ip6.
  */
-static int addr_is_ip6(struct sockaddr_storage* addr)
+static int 
+addr_is_ip6(struct sockaddr_storage* addr)
 {
 	short family = *(short*)addr;
 	if(family == AF_INET6)
@@ -402,7 +413,8 @@ static int addr_is_ip6(struct sockaddr_storage* addr)
  * @param outnet: network structure that has arrays of ports to choose from.
  * @param pend: the message to send. c is filled in, randomly chosen.
  */
-static void select_port(struct outside_network* outnet, struct pending* pend)
+static void 
+select_port(struct outside_network* outnet, struct pending* pend)
 {
 	double precho;
 	int chosen, nummax;
@@ -438,14 +450,15 @@ static void select_port(struct outside_network* outnet, struct pending* pend)
 }
 
 
-void pending_udp_query(struct outside_network* outnet, ldns_buffer* packet, 
+void 
+pending_udp_query(struct outside_network* outnet, ldns_buffer* packet, 
 	struct sockaddr_storage* addr, socklen_t addrlen, int timeout,
 	comm_point_callback_t* cb, void* cb_arg)
 {
 	struct pending* pend;
 	struct timeval tv;
 
-	/* create pending struct (and possibly change ID to be unique) */
+	/* create pending struct and change ID to be unique */
 	if(!(pend=new_pending(outnet, packet, addr, addrlen, cb, cb_arg))) {
 		/* callback user for the error */
 		(void)(*cb)(NULL, cb_arg, NETEVENT_CLOSED, NULL);
@@ -456,10 +469,9 @@ void pending_udp_query(struct outside_network* outnet, ldns_buffer* packet,
 	/* send it over the commlink */
 	if(!comm_point_send_udp_msg(pend->c, packet, (struct sockaddr*)addr, 
 		addrlen)) {
-		/* error, call error callback function */
-		pending_delete(outnet, pend);
 		/* callback user for the error */
 		(void)(*pend->cb)(pend->c, pend->cb_arg, NETEVENT_CLOSED, NULL);
+		pending_delete(outnet, pend);
 		return;
 	}
 
diff --git a/services/outside_network.h b/services/outside_network.h
index 4659238db..06d55886d 100644
--- a/services/outside_network.h
+++ b/services/outside_network.h
@@ -108,12 +108,13 @@ struct pending {
  * @param bufsize: size for network buffers.
  * @param num_ports: number of udp ports to open per interface.
  * @param ifs: interface names (or NULL for default interface).
+ *    These interfaces must be able to access all authoritative servers.
  * @param num_ifs: number of names in array ifs.
  * @param do_ip4: service IP4.
  * @param do_ip6: service IP6.
  * @param port_base: if -1 system assigns ports, otherwise try to get
  *    the ports numbered from this starting number.
- * @return: the new empty structure or NULL on error.
+ * @return: the new structure (with no pending answers) or NULL on error.
  */
 struct outside_network* outside_network_create(struct comm_base* base,
 	size_t bufsize, size_t num_ports, const char** ifs, int num_ifs,
@@ -127,12 +128,15 @@ void outside_network_delete(struct outside_network* outnet);
 
 /**
  * Send UDP query, create pending answer.
+ * Changes the ID for the query to be random and unique for that destination.
  * @param outnet: provides the event handling
  * @param packet: wireformat query to send to destination.
  * @param addr: address to send to.
  * @param addrlen: length of addr.
  * @param timeout: in seconds from now.
  * @param callback: function to call on error, timeout or reply.
+ *    The routine does not return an error, instead it calls the callback,
+ *    with an error code if an error happens.
  * @param callback_arg: user argument for callback function.
  */
 void pending_udp_query(struct outside_network* outnet, ldns_buffer* packet, 
@@ -142,7 +146,7 @@ void pending_udp_query(struct outside_network* outnet, ldns_buffer* packet,
 /**
  * Delete pending answer.
  * @param outnet: outside network the pending query is part of.
- *    Used internal, if NULL, p is not unlinked from rbtree.
+ *    Internal feature: if outnet is NULL, p is not unlinked from rbtree.
  * @param p: deleted
  */
 void pending_delete(struct outside_network* outnet, struct pending* p);
@@ -150,7 +154,7 @@ void pending_delete(struct outside_network* outnet, struct pending* p);
 /** 
  * See if string is ip4 or ip6.
  * @param str: IP specification.
- * @return: true is string addr is an ip6 specced address. 
+ * @return: true if string addr is an ip6 specced address. 
  */
 int str_is_ip6(const char* str);
 
diff --git a/testcode/testbound.c b/testcode/testbound.c
index 47329e703..3348717fd 100644
--- a/testcode/testbound.c
+++ b/testcode/testbound.c
@@ -86,6 +86,7 @@ add_opts(char* optarg, int* pass_argc, char* pass_argv[])
 		else	len = strlen(p);
 		/* allocate and copy option */
 		if(*pass_argc >= MAXARG-1) {
+			/* printf because log_init is not yet called. */
 			printf("too many arguments: '%s'\n", p);
 			exit(1);
 		}
@@ -121,7 +122,8 @@ echo_cmdline(int argc, char* argv[])
  * @param argc: arg count.
  * @param argv: array of commandline arguments.
  */
-int main(int argc, char* argv[])
+int 
+main(int argc, char* argv[])
 {
 	int c, res;
 	int pass_argc = 0;
diff --git a/testcode/unitmain.c b/testcode/unitmain.c
index c9a53895a..6af9436cc 100644
--- a/testcode/unitmain.c
+++ b/testcode/unitmain.c
@@ -47,9 +47,10 @@ int testcount = 0;
 /** test bool x, exits on failure, increases testcount. */
 #define unit_assert(x) testcount++; log_assert(x);
 
-/** test net code */
 #include "services/outside_network.h"
-static void net_test()
+/** test net code */
+static void 
+net_test()
 {
 	unit_assert( str_is_ip6("::") );
 	unit_assert( str_is_ip6("::1") );
@@ -66,7 +67,8 @@ static void net_test()
  * @param argc: arg count.
  * @param argv: array of commandline arguments.
  */
-int main(int argc, char* argv[])
+int 
+main(int argc, char* argv[])
 {
 	if(argc != 1) {
 		printf("usage: %s\n", argv[0]);