removed debugging cruft, added README and stub headers for standalone testing

diff --git a/Makefile b/Makefile
index c479c1b0af5654e9cae89166d18b6f1cfb950669..cc5166c5de0c503273aa7adb0679f7a97467e9f8 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -5,8 +5,8 @@ LDFLAGS += -lpthread
 MAKEDEPEND = $(CC) -MM
 
 TARGETS = resolver
 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
 OBJECTS = $(SOURCES:.c=.o)
 
 .phony:        all test clean

diff --git a/README b/README
new file mode 100644 (file)
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 (file)
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 (file)
index 0000000..e69de29

diff --git a/lru_cache_test.c b/lru_cache_test.c
deleted file mode 100644 (file)
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 50907e3ef07b0d5312153bea17c684319f61d8fc..a78f0f068813777d40acde7da86915e9f87dff05 100644 (file)
--- 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 */
 #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
 
 
 static


@@ -79,8 +79,8 @@ struct options {
 
 /*
        N.B.  The hostname in a cache entry is a flexible array!
 
 /*
        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 */
 */
 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;
 
        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);
        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 */
 
        }
 #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 ) {
        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
        }
 
 #ifdef LOG_DEBUG

+       /* forces muck-spawned resolver to log debug messages */

        char lf[] = "/tmp/resolver-debug.XXXXXX";
        int lf_fd = mkstemp(lf);
 
        char lf[] = "/tmp/resolver-debug.XXXXXX";
        int lf_fd = mkstemp(lf);