From: Wouter Wijngaards Date: Tue, 6 Feb 2007 14:00:52 +0000 (+0000) Subject: Review results. for util/ X-Git-Tag: release-0.0~56 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=74ec9e6553b9fd9cf060487ba55c10bdc830d3d2;p=thirdparty%2Funbound.git 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 476dc4278..eec2b252a 100644 --- 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 3afc8fa52..45291e97a 100644 --- 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 0c36dbbb0..8fe90934e 100644 --- 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 5b2d81a96..0b6eb6c24 100644 --- a/util/netevent.c +++ b/util/netevent.c @@ -44,6 +44,7 @@ #include /* -------- Start of local definitions -------- */ +/* We define libevent structures here to hide the libevent stuff. */ /* we use libevent */ #include @@ -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 bca68557c..f6e045398 100644 --- 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 76cef3df1..9b1730fe6 100644 --- 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 1507eb302..aaa176995 100644 --- 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) \