From d9735e821e3631900debe55a09c5e1e38078b89f Mon Sep 17 00:00:00 2001
From: Justin Wind <justin.wind+git@gmail.com>
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 <stdlib.h>
-#include <unistd.h>
-#include <stdio.h>
-#include <assert.h>
-
-/* 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.49.0