From d9735e821e3631900debe55a09c5e1e38078b89f Mon Sep 17 00:00:00 2001 From: Justin Wind Date: Wed, 10 Sep 2014 16:33:44 -0700 Subject: [PATCH] removed debugging cruft, added README and stub headers for standalone testing --- Makefile | 4 +- README | 29 +++++++ config.h | 5 ++ copyright.h | 0 lru_cache_test.c | 198 ----------------------------------------------- resolver.c | 23 +----- 6 files changed, 40 insertions(+), 219 deletions(-) create mode 100644 README create mode 100644 config.h create mode 100644 copyright.h delete mode 100644 lru_cache_test.c diff --git a/Makefile b/Makefile index c479c1b..cc5166c 100644 --- a/Makefile +++ b/Makefile @@ -5,8 +5,8 @@ LDFLAGS += -lpthread MAKEDEPEND = $(CC) -MM TARGETS = resolver -TESTS = lru_cache_test -SOURCES = lru_cache.c lru_cache_test.c resolver.c +TESTS = +SOURCES = lru_cache.c resolver.c OBJECTS = $(SOURCES:.c=.o) .phony: all test clean diff --git a/README b/README new file mode 100644 index 0000000..6a8a676 --- /dev/null +++ b/README @@ -0,0 +1,29 @@ + +This began as a cleanup of a few startling bugs in resolver.c, but I encountered so many other endemic issues (IMO) along the way that I just started from the top and groomed everything on my way through. Apologies if the style doesn't quite match the rest of the codebase, but I hope it's at least a little easier to follow. + +The resolver is still architected the same as the original (id est, a group of threads, all feeding from a common input stream in turn, resolving their data and responding independently before repeating), but it strives to do the things it does a little more correctly. + +I've probably screwed up portability somewhere, and it may now require a non-vintage compiler. + + +Major fixes: + + * locking. original had none around its core data structures (!) + * functions no longer harbor static buffers (!) + * non-blocking IO is more resilient + + +Other changes: + + * uses modern socket framework + * can cache successful lookups as well as failures + * cache lookups are now less than linear + * uses native stdio locking + + +Possible future improvements to consider: + + * track more internal data, dump stats on a signal + * make the threadpool dynamically sized + * use a different work-queue model entirely + diff --git a/config.h b/config.h new file mode 100644 index 0000000..c94299a --- /dev/null +++ b/config.h @@ -0,0 +1,5 @@ +#define USE_IPV6 +#define WITH_RESOLVER_STATS +#define HAVE_GETRLIMIT + +#undef LOG_DEBUG diff --git a/copyright.h b/copyright.h new file mode 100644 index 0000000..e69de29 diff --git a/lru_cache_test.c b/lru_cache_test.c deleted file mode 100644 index 135ec34..0000000 --- a/lru_cache_test.c +++ /dev/null @@ -1,198 +0,0 @@ -#include -#include -#include -#include - -/* wrap the whole module */ -#include "lru_cache.c" - -static unsigned int g_verbose_ = 0; - -struct test_kv_cache_entry { - struct lru_cache_entry lru_; - - char key[128]; - char value[128]; -}; -typedef struct test_kv_cache_entry kve_t; - -static -void -test_kv_hash_feed(lru_entry_t *entry, char **bufp, size_t *szp) -{ - struct test_kv_cache_entry *e = (struct test_kv_cache_entry *)entry; - *bufp = e->key; - *szp = sizeof e->key; -} - -static -int -test_kv_cmp(lru_entry_t *a, lru_entry_t *b) -{ - kve_t *kv_a = (kve_t *)a, - *kv_b = (kve_t *)b; - - return strcmp(kv_a->key, kv_b->key); -} - -static -void -kv_dump(struct lru_cache *c) -{ - struct test_kv_cache_entry *kve; - lru_entry_t *e; - size_t i; - - fprintf(stderr, "\nLRU, %zu entries\n", c->num_entries); - - if (g_verbose_ == 0) - return; - - fprintf(stderr, "-- hash --\n"); - for (i = 0; i < c->hash_sz; i++) { - int s; - for (s = 0, e = c->hash[i].entry; e; e = e->hash_next, s++) { - kve = (struct test_kv_cache_entry *)e; - if (e != NULL) { - if (e->hash_slot != i) - fprintf(stderr, "!!! sanity failure, entry has hash slot %zu, but is in slot %zu !!!\n", e->hash_slot, i); - fprintf(stderr, "%*s[%zu]: (%p) '%s':'%s'\n", s, "", i, kve, kve->key, kve->value); - } - } - } - - fprintf(stderr, "-- queue --\n"); - for (e = c->newest; e; e = e->next) { - kve = (struct test_kv_cache_entry *)e; - fprintf(stderr, "(%p) '%s':'%s'\n", kve, kve->key, kve->value); - } - - fprintf(stderr, "\n"); -} - -void -kv_genstats(lru_entry_t *e, size_t i, void *data) -{ - struct test_kv_cache_entry *kve = (struct test_kv_cache_entry *)e; - size_t *hash_depth = (size_t *)data; - (void)i, (void)kve; - - hash_depth[e->hash_slot]++; -} -void -kv_dumpstats(struct lru_cache *c, size_t *stats, size_t sz) -{ - size_t i; - - for (i = 0; i < sz; i++) - fprintf(stderr, "slot %04zu: %zu (expect: %zu)\n", i, stats[i], c->hash[i].tally); -} - -int -main(int argc, char *argv[]) -{ - const size_t hash_sz = 31; - const size_t capacity = 256; - - struct lru_cache *cache; - struct test_kv_cache_entry *e, *r, m; - int i; - - size_t *stats; - - while ( (i = getopt(argc, argv, "vh")) != EOF ) { - switch (i) { - case 'v': - g_verbose_++; - break; - - case 'h': - default: - exit(EXIT_FAILURE); - } - } - - stats = malloc(hash_sz * sizeof *stats); - memset(stats, 0, hash_sz * sizeof *stats); - - cache = lru_cache_new(hash_sz, capacity, test_kv_hash_feed, test_kv_cmp); - assert(cache != NULL); - - kv_dump(cache); - - e = malloc(sizeof *e); - assert(e != NULL); - - strncpy(e->key, "key", sizeof e->key); - strncpy(e->value, "value", sizeof e->value); - - lru_cache_insert(cache, (lru_entry_t *)e, (lru_entry_t **)&r); - assert(r == NULL); - - kv_dump(cache); - - memset(&m, 0, sizeof m); - strncpy(m.key, "key", sizeof e->key); - e = (struct test_kv_cache_entry *)lru_cache_locate(cache, (lru_entry_t *)&m); - assert(e != NULL); - - lru_cache_extract(cache, (lru_entry_t *)e); - free(e); - - kv_dump(cache); - - for (i = 0; i < 500; i++) { - r = NULL; - e = malloc(sizeof *e); - assert(e != NULL); - memset(e, 0, sizeof *e); - snprintf(e->key, sizeof e->key, "test %d key", i); - snprintf(e->value, sizeof e->value, "some %d value", i); - lru_cache_insert(cache, (lru_entry_t *)e, (lru_entry_t **)&r); - if (r) { - if (g_verbose_) - fprintf(stderr, "freeing (%p) '%s':'%s'\n", r, r->key, r->value); - free(r); - } - } - - kv_dump(cache); - - for (i = 0; i < 500; i += 3) { - memset(&m, 0, sizeof m); - snprintf(m.key, sizeof m.key, "test %d key", i); - e = (struct test_kv_cache_entry *)lru_cache_locate(cache, (lru_entry_t *)&m); - if (e == NULL) { - if (g_verbose_) - fprintf(stderr, "key %d was not cached\n", i); - continue; - } - lru_cache_extract(cache, (lru_entry_t *)e); - if (g_verbose_) - fprintf(stderr, "extracted '%s'\n", e->key); - free(e); - } - - kv_dump(cache); - - for (i = 0; i < 10; i += 2) { - e = malloc(sizeof *e); - assert(e != NULL); - memset(e, 0, sizeof *e); - snprintf(e->key, sizeof e->key, "new key %d", i); - snprintf(e->value, sizeof e->value, "new value %d", i); - lru_cache_insert(cache, (lru_entry_t *)e, (lru_entry_t **)&r); - if (r) { - if (g_verbose_) - fprintf(stderr, "freeing (%p) '%s':'%s'\n", r, r->key, r->value); - free(r); - } - } - - kv_dump(cache); - - lru_cache_foreach(cache, kv_genstats, stats); - kv_dumpstats(cache, stats, hash_sz); - - exit(EXIT_SUCCESS); -} diff --git a/resolver.c b/resolver.c index 50907e3..a78f0f0 100644 --- a/resolver.c +++ b/resolver.c @@ -54,7 +54,7 @@ #define QUIT_COMMAND "QUIT" /* shut down if this input is encountered */ #ifdef WITH_RESOLVER_STATS # define DUMP_COMMAND "DUMP" /* report on internal state */ -#endif +#endif /* WITH_RESOLVER_STATS */ static @@ -79,8 +79,8 @@ struct options { /* N.B. The hostname in a cache entry is a flexible array! - If this is ever updated to c99 style, also update how it's allocated - in cache_add_. + If this is ever updated to c99 style, also update how it's + allocated in cache_add_. */ struct cache_entry { lru_entry_t lru_; /* must be first */ @@ -760,18 +760,6 @@ ident_query_(char *out_buf, size_t out_buf_sz, struct sockaddr_storage *ss, unsi unsigned short *vport; socklen_t sockaddr_len; -#ifdef DEBUG_PRETEND_IDENT - size_t delay = lrand48() % IDENT_TIMEOUT; - if (delay < 45) { - delay = delay / 5; - sleep(delay); - } else { - delay = 0; - } - LOG_VVV("pretending ident query took %zu seconds\n", delay); - goto done; -#endif /* DEBUG_PRETEND_IDENT */ - deadline = time(&now) + IDENT_TIMEOUT; fd = socket(ss->ss_family, SOCK_STREAM, 0); @@ -1139,10 +1127,6 @@ main(int argc, char **argv) } #endif /* HAVE_GETRLIMIT */ -#ifdef DEBUG_PRETEND_IDENT - srand48(getpid()); -#endif - g_log_stream_ = stderr; while ( (c = getopt(argc, argv, "a:c:f:j:s:o:vh")) != EOF ) { @@ -1205,6 +1189,7 @@ main(int argc, char **argv) } #ifdef LOG_DEBUG + /* forces muck-spawned resolver to log debug messages */ char lf[] = "/tmp/resolver-debug.XXXXXX"; int lf_fd = mkstemp(lf); -- 2.45.2