Review results. for util/

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

diff --git a/daemon/unbound.c b/daemon/unbound.c
index 476dc4278b84022c7d2f44e0038a2d27472510e1..eec2b252a65997bdefe0836abce01fd8eeafb4de 100644 (file)
--- a/daemon/unbound.c
+++ b/daemon/unbound.c
@@ -85,6 +85,7 @@ main(int argc, char* argv[])
        const char* fwdport = UNBOUND_DNS_PORT;
 
        log_init();
+       log_info("Start of %s.", PACKAGE_STRING);
        /* parse the options */
        while( (c=getopt(argc, argv, "f:hvp:z:")) != -1) {
                switch(c) {
@@ -131,7 +132,7 @@ main(int argc, char* argv[])
        }
        
        /* drop user priviliges and chroot if needed */
-       log_info("Start of %s.", PACKAGE_STRING);
+       verbose(VERB_OPS, "start of service (%s).", PACKAGE_STRING);
        worker_work(worker);
 
        /* cleanup */

diff --git a/doc/Changelog b/doc/Changelog
index 3afc8fa52dedbc88844114bbb828ef138f97286c..45291e97af292c5e0e2d826bddd19da2c522bd31 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,3 +1,6 @@
+6 February 2007: Wouter
+       - reviewed code and improved in places.
+
 5 February 2007: Wouter
        - Picked up stdc99 and other define tests from ldns. Improved
          POSIX define test to include getaddrinfo.

diff --git a/util/log.h b/util/log.h
index 0c36dbbb0bf04b80ab80d436f7a5e313d834d0a5..8fe90934e9cf60b1fc40e80065556eeac505df47 100644 (file)
--- a/util/log.h
+++ b/util/log.h
@@ -115,7 +115,7 @@ void log_vmsg(const char* type, const char* format, va_list args);
  */
 #ifdef UNBOUND_ASSERT
 #  define log_assert(x) \
-       do { if(!(x)) \
+       do { if(!(x)) \
                fatal_exit("%s:%d: %s: assertion %s failed", \
                        __FILE__, __LINE__, __func__, #x); \
        } while(0);

diff --git a/util/netevent.c b/util/netevent.c
index 5b2d81a96e2e91185d36f04f666845a60b4b7db7..0b6eb6c245ea1c36d5cd6f5a51c079f6bb026988 100644 (file)
--- a/util/netevent.c
+++ b/util/netevent.c
@@ -44,6 +44,7 @@
 #include <errno.h>
 
 /* -------- Start of local definitions -------- */
+/* We define libevent structures here to hide the libevent stuff. */
 
 /* we use libevent */
 #include <event.h>
@@ -71,7 +72,7 @@ struct internal_base {
 struct internal_timer {
        /** libevent event type, alloced here */
        struct event ev;
-       /** is timer enabled, yes or no */
+       /** is timer enabled */
        uint8_t enabled;
 };
 
@@ -81,8 +82,6 @@ struct internal_timer {
 struct internal_signal {
        /** libevent event type, alloced here */
        struct event ev;
-       /** the commpoint it is part of */
-       struct comm_signal* comm;
        /** next in signal list */
        struct internal_signal* next;
 };
@@ -125,7 +124,7 @@ static void comm_timer_callback(int fd, short event, void* arg);
 
 /**
  * handle libevent callback for signal comm.
- * @param fd: file descriptor (used signal number).
+ * @param fd: file descriptor (used for the signal number).
  * @param event: event bits from libevent: 
  *     EV_READ, EV_WRITE, EV_SIGNAL, EV_TIMEOUT.
  * @param arg: the internal commsignal structure.
@@ -218,6 +217,7 @@ comm_point_udp_callback(int fd, short event, void* arg)
        ssize_t recv;
 
        rep.c = (struct comm_point*)arg;
+       log_assert(rep.c->type == comm_udp);
 
        verbose(VERB_ALGO, "callback udp");
        if(!(event&EV_READ))
@@ -248,6 +248,8 @@ comm_point_tcp_accept_callback(int ATTR_UNUSED(fd), short ATTR_UNUSED(event),
 {
        struct comm_point* c = (struct comm_point*)arg;
        log_info("callback tcpaccept for %x", (int)c);
+       log_assert(c->type == comm_tcp_accept);
+       /* TODO */
 }
 
 static void 
@@ -256,6 +258,8 @@ comm_point_tcp_handle_callback(int ATTR_UNUSED(fd), short ATTR_UNUSED(event),
 {
        struct comm_point* c = (struct comm_point*)arg;
        log_info("callback tcpaccept for %x", (int)c);
+       log_assert(c->type == comm_tcp);
+       /* TODO */
 }
 
 struct comm_point* 
@@ -375,6 +379,11 @@ comm_point_create_tcp(struct comm_base *base, int fd, int num, size_t bufsize,
        c->max_tcp_count = num;
        c->tcp_handlers = (struct comm_point**)calloc((size_t)num,
                sizeof(struct comm_point*));
+       if(!c->tcp_handlers) {
+               free(c->ev);
+               free(c);
+               return NULL;
+       }
        c->tcp_free = NULL;
        c->type = comm_tcp_accept;
        c->tcp_do_close = 0;
@@ -384,16 +393,11 @@ comm_point_create_tcp(struct comm_base *base, int fd, int num, size_t bufsize,
        evbits = EV_READ | EV_PERSIST;
        /* libevent stuff */
        event_set(&c->ev->ev, c->fd, evbits, comm_point_tcp_accept_callback, c);
-       if( event_base_set(base->eb->base, &c->ev->ev) != 0 ||
+       if(event_base_set(base->eb->base, &c->ev->ev) != 0 ||
                event_add(&c->ev->ev, c->timeout) != 0 )
        {
                log_err("could not add tcpacc event");
-               if(!event_del(&c->ev->ev)) {
-                       log_err("could not event_del tcpacc event");
-               }
-               free(c->tcp_handlers);
-               free(c->ev);
-               free(c);
+               comm_point_delete(c);
                return NULL;
        }
 
@@ -413,10 +417,12 @@ comm_point_create_tcp(struct comm_base *base, int fd, int num, size_t bufsize,
 void 
 comm_point_close(struct comm_point* c)
 {
+       if(!c)
+               return;
        if(event_del(&c->ev->ev) != 0) {
                log_err("could not event_del on close");
        }
-       /* close fd after removing from event lists, or epoll/etc messes up */
+       /* close fd after removing from event lists, or epoll.. is messed up */
        if(c->fd != -1)
                close(c->fd);
        c->fd = -1;
@@ -447,7 +453,8 @@ comm_point_set_cb_arg(struct comm_point* c, void *arg)
        c->cb_arg = arg;
 }
 
-void comm_point_send_reply(struct comm_reply *repinfo)
+void 
+comm_point_send_reply(struct comm_reply *repinfo)
 {
        log_assert(repinfo && repinfo->c);
        if(repinfo->c->type == comm_udp) {
@@ -455,11 +462,12 @@ void comm_point_send_reply(struct comm_reply *repinfo)
                        (struct sockaddr*)&repinfo->addr, repinfo->addrlen);
        } else {
                log_info("tcp reply");
+               /* TODO */
        }
 }
 
-struct comm_timer* comm_timer_create(struct comm_base* base,
-        void (*cb)(void*), void* cb_arg)
+struct comm_timer* 
+comm_timer_create(struct comm_base* base, void (*cb)(void*), void* cb_arg)
 {
        struct comm_timer *tm = (struct comm_timer*)calloc(1,
                sizeof(struct comm_timer));
@@ -474,7 +482,6 @@ struct comm_timer* comm_timer_create(struct comm_base* base,
        }
        tm->callback = cb;
        tm->cb_arg = cb_arg;
-       /*evtimer_set(&tm->ev_timer->ev, comm_timer_callback, tm);*/
        event_set(&tm->ev_timer->ev, -1, EV_PERSIST|EV_TIMEOUT, 
                comm_timer_callback, tm);
        if(event_base_set(base->eb->base, &tm->ev_timer->ev) != 0) {
@@ -486,7 +493,8 @@ struct comm_timer* comm_timer_create(struct comm_base* base,
        return tm;
 }
 
-void comm_timer_disable(struct comm_timer* timer)
+void 
+comm_timer_disable(struct comm_timer* timer)
 {
        if(!timer)
                return;
@@ -494,7 +502,8 @@ void comm_timer_disable(struct comm_timer* timer)
        timer->ev_timer->enabled = 0;
 }
 
-void comm_timer_set(struct comm_timer* timer, struct timeval* tv)
+void 
+comm_timer_set(struct comm_timer* timer, struct timeval* tv)
 {
        if(timer->ev_timer->enabled)
                comm_timer_disable(timer);
@@ -502,7 +511,8 @@ void comm_timer_set(struct comm_timer* timer, struct timeval* tv)
        timer->ev_timer->enabled = 1;
 }
 
-void comm_timer_delete(struct comm_timer* timer)
+void 
+comm_timer_delete(struct comm_timer* timer)
 {
        if(!timer)
                return;
@@ -527,7 +537,8 @@ comm_timer_is_set(struct comm_timer* timer)
        return (int)timer->ev_timer->enabled;
 }
 
-struct comm_signal* comm_signal_create(struct comm_base* base,
+struct comm_signal* 
+comm_signal_create(struct comm_base* base,
         void (*callback)(int, void*), void* cb_arg)
 {
        struct comm_signal* com = (struct comm_signal*)malloc(
@@ -543,15 +554,17 @@ struct comm_signal* comm_signal_create(struct comm_base* base,
        return com;
 }
 
-static void comm_signal_callback(int sig, short event, void* arg)
+static void 
+comm_signal_callback(int sig, short event, void* arg)
 {
-       struct internal_signal* entry = (struct internal_signal*)arg;
+       struct comm_signal* comsig = (struct comm_signal*)arg;
        if(!(event & EV_SIGNAL))
                return;
-       (*entry->comm->callback)(sig, entry->comm->cb_arg);
+       (*comsig->callback)(sig, comsig->cb_arg);
 }
 
-int comm_signal_bind(struct comm_signal* comsig, int sig)
+int 
+comm_signal_bind(struct comm_signal* comsig, int sig)
 {
        struct internal_signal* entry = (struct internal_signal*)calloc(1, 
                sizeof(struct internal_signal));
@@ -560,9 +573,8 @@ int comm_signal_bind(struct comm_signal* comsig, int sig)
                return 0;
        }
        log_assert(comsig);
-       entry->comm = comsig;
        /* add signal event */
-       signal_set(&entry->ev, sig, comm_signal_callback, entry);
+       signal_set(&entry->ev, sig, comm_signal_callback, comsig);
        if(event_base_set(comsig->base->eb->base, &entry->ev) != 0) {
                log_err("Could not set signal base");
                free(entry);
@@ -579,7 +591,8 @@ int comm_signal_bind(struct comm_signal* comsig, int sig)
        return 1;
 }
 
-void comm_signal_delete(struct comm_signal* comsig)
+void 
+comm_signal_delete(struct comm_signal* comsig)
 {
        struct internal_signal* p, *np;
        if(!comsig)

diff --git a/util/netevent.h b/util/netevent.h
index bca68557cc0c00cdd6489fc71da932a6ac27459c..f6e045398e9ab3ba986e94ae61fdc7b1ab7df1b2 100644 (file)
--- a/util/netevent.h
+++ b/util/netevent.h
@@ -48,6 +48,13 @@
  *    o frontside - aimed towards our clients, queries come in, answers back.
  *    o behind - aimed towards internet, to the authoritative DNS servers.
  *
+ * Several event types are available:
+ *    o comm_base - for thread safety of the comm points, one per thread.
+ *    o comm_point - udp and tcp networking, with callbacks.
+ *    o comm_timer - a timeout with callback.
+ *    o comm_signal - callbacks when signal is caught.
+ *    o comm_reply - holds reply info during networking callback.
+ *
  */
 
 #ifndef NET_EVENT_H
@@ -158,6 +165,7 @@ struct comm_point {
            later time. It consists of a struct with commpoint and address.
            It can be passed to a msg send routine some time later.
            Note the reply information is temporary and must be copied.
+           NULL is passed for_reply info, in cases where error happened.
 
            declare as: 
            int my_callback(struct comm_point* c, void* my_arg, int error,
@@ -351,17 +359,18 @@ void comm_timer_delete(struct comm_timer* timer);
 int comm_timer_is_set(struct comm_timer* timer);
 
 /**
- * Create a signal handler.
+ * Create a signal handler. Call signal_bind() later to bind to a signal.
  * @param base: communication base to use.
  * @param callback: called when signal is caught.
  * @param cb_arg: user argument to callback
- * @return: the signal struct (bind it to a signal) or NULL on error.
+ * @return: the signal struct or NULL on error.
  */
 struct comm_signal* comm_signal_create(struct comm_base* base,
        void (*callback)(int, void*), void* cb_arg);
 
 /**
- * Bind signal struct to catch a signal.
+ * Bind signal struct to catch a signal. A signle comm_signal can be bound
+ * to multiple signals, calling comm_signal_bind multiple times.
  * @param comsig: the communication point, with callback information.
  * @param sig: signal number.
  * @return: true on success. false on error.

diff --git a/util/rbtree.c b/util/rbtree.c
index 76cef3df1045a46a3770a3dbc08dd6a0b34b83ee..9b1730fe62de7ca1b65d6c1c02f003b96861da11 100644 (file)
--- a/util/rbtree.c
+++ b/util/rbtree.c
@@ -1,9 +1,7 @@
 /*
  * rbtree.c -- generic red black tree
  *
- * Copyright (c) 2001-2006, NLnet Labs. All rights reserved.
- *
- * Copyright (c) 2007, NLnet Labs. All rights reserved.
+ * Copyright (c) 2001-2007, NLnet Labs. All rights reserved.
  * 
  * This software is open source.
  * 

diff --git a/util/rbtree.h b/util/rbtree.h
index 1507eb30257caeeec6d2637e2f8c9cffef176920..aaa1769955820f04000695eba4a68fdff356815e 100644 (file)
--- a/util/rbtree.h
+++ b/util/rbtree.h
@@ -1,9 +1,7 @@
 /*
  * rbtree.h -- generic red-black tree
  *
- * Copyright (c) 2001-2006, NLnet Labs. All rights reserved.
- *
- * Copyright (c) 2007, NLnet Labs. All rights reserved.
+ * Copyright (c) 2001-2007, NLnet Labs. All rights reserved.
  * 
  * This software is open source.
  * 
@@ -52,7 +50,7 @@
  */
 typedef struct rbnode_t rbnode_t;
 /**
- * The rbnore_t struct definition.
+ * The rbnode_t struct definition.
  */
 struct rbnode_t {
        /** parent in rbtree, RBTREE_NULL for root */
@@ -173,7 +171,7 @@ rbnode_t *rbtree_previous(rbnode_t *rbtree);
                ((d) = (void *) (rbtree)->_node); (rbtree)->_node = rbtree_next((rbtree)->_node))
 
 /**
- * call with node=variable of struct* with rbnode_t as first element.
+ * Call with node=variable of struct* with rbnode_t as first element.
  * with type is the type of a pointer to that struct. 
  */
 #define RBTREE_FOR(node, type, rbtree) \